forked from ethereum-optimism/optimism
-
Notifications
You must be signed in to change notification settings - Fork 1
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
test: superc20 tob properties #27
Merged
0xteddybear
merged 7 commits into
test/state-transitions
from
test/superc20-tob-properties
Aug 26, 2024
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
612c137
chore: add crytic/properties dependency
0xteddybear 4893fba
test: extend protocol properties so it also covers ToB erc20 properties
0xteddybear 64b59fb
chore: small linter fixes
0xteddybear dfd25cb
docs: update property list
0xteddybear 50f6e2e
test: handlers for remaining superc20 state transitions
0xteddybear 1cbdfab
fix: disable ToB properties we are not using and guide the fuzzer a b…
0xteddybear 3b1af6c
fix: disable another ToB property not implemented by solady
0xteddybear File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Submodule properties
added at
bb1b78
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 9 additions & 0 deletions
9
...s/contracts-bedrock/test/properties/helpers/OptimismSuperchainERC20ForToBProperties.t.sol
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
// SPDX-License-Identifier: AGPL-3 | ||
pragma solidity ^0.8.25; | ||
|
||
import { OptimismSuperchainERC20 } from "src/L2/OptimismSuperchainERC20.sol"; | ||
|
||
contract OptimismSuperchainERC20ForToBProperties is OptimismSuperchainERC20 { | ||
bool public constant isMintableOrBurnable = true; | ||
uint256 public initialSupply = 0; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this isn't an assumption somewhere else in the codebase, right (like in a mint or burn or smth)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, it's not checked anywhere else and I didn't find it in the docs either. Perhaps it's worth checking with @0xParticle :
the above implies there will not be a bijection between
OptimismSuperchainERC20
Transfer({from: somebody, to: address(0), amount: something})
events andsendERC20
callsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually an historical discussion. The ERC20 standard does not mention anything about these scenarios, so implementation can block or not transfers to the zero address. Same for tracking the balance of the zero address.
Also, keep in mind that transfers to the zero address have a use case, which is to effectively burn tokens.
So, answering the 3 bullet points, the three scenarios are ok as its ERC20 compliant.
Regarding the symmetry between transfers and bridging, I think its fine.
transfer
andtransferFrom
methods in the solady implementation. I don't see why someone would do it via a bridge, unless they want to waste gas.