Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update docs for SafeERC20.forceApprove #4231

Merged
merged 1 commit into from
Jul 1, 2023

Conversation

PaulRBerg
Copy link
Contributor

I was reading the release notes of v4.9.0-rc.0, when I saw this PR mentioned:

#4067

This, in turn, led me to this PR:

#3851

I read all the comments there, but I was still unsure what the goal of forceApprove is. Then, I looked at USDT's source code, and it all became clear to me.

This PR rewrites the NatSpec of SafeERC20.forceApprove to mention USDT since Tether is the primary use case for this function.

@changeset-bot
Copy link

changeset-bot bot commented May 10, 2023

🦋 Changeset detected

Latest commit: 432deac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
openzeppelin-solidity Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@frangio frangio changed the title docs: update NatSpec in SafeERC20.forceApprove Update docs for SafeERC20.forceApprove Jul 1, 2023
@frangio frangio force-pushed the docs/force-approve branch from 961adf2 to 432deac Compare July 1, 2023 04:33
@frangio frangio changed the base branch from release-v4.9 to master July 1, 2023 04:33
Copy link
Contributor

@frangio frangio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @PaulRBerg!

@frangio frangio merged commit 06861dc into OpenZeppelin:master Jul 1, 2023
frangio pushed a commit that referenced this pull request Jul 1, 2023
@PaulRBerg PaulRBerg deleted the docs/force-approve branch July 1, 2023 04:46
nicholaspai added a commit to across-protocol/contracts that referenced this pull request Sep 5, 2024
…pgrade

Imports an [aliased npm package](https://forum.openzeppelin.com/t/coexist-of-v5-and-v4-contracts/38030/5) so we can use multiple oz libraries in the same solidity contracts.

This way we get access to new and improved contracts. I make changes to all contracts that we can upgrade, such as SpokePools and NOT the HubPool.

For example, I can use the new SafeERC20 method `forceApprove` instead of `safeIncreaseAllowance` which ensures [compatibility with tokens like USDT](OpenZeppelin/openzeppelin-contracts#4231) that make sure all approvals are set to 0 before granting a new approval. This hasn't been an issue so far because we always safeIncreaseAllowance to some number and use the complete allowance, but its worth safety checking.

Other changes:
- Moved `MerkleDistributor` out of uma/core into this repo to reduce dependency on this external repo
- Replaced isCode() call with now [recommended](OpenZeppelin/openzeppelin-contracts#3945) explicit .code.length check
- Explicitly set SpokePoolVerifier pragma to 0.8.19 and removed the overrides in hardhat.config.ts
- Removed unused import in PolygonERC20Test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants