Added hardfork support + deterministic state to reference implementation#31
Open
jaydenwindle wants to merge 2 commits intomainfrom
Open
Added hardfork support + deterministic state to reference implementation#31jaydenwindle wants to merge 2 commits intomainfrom
jaydenwindle wants to merge 2 commits intomainfrom
Conversation
…pport for post-hardfork account usage
Vectorized
reviewed
Apr 11, 2024
| function owner() public view virtual returns (address) { | ||
| (uint256 chainId, address tokenContract, uint256 tokenId) = token(); | ||
| if (chainId != block.chainid) return address(0); | ||
| if (chainId != block.chainid && block.chainid == deploymentChainId) return address(0); |
Contributor
There was a problem hiding this comment.
Shouldn't this be chainId != block.chainid && chainid != deploymentChainId?
Vectorized
reviewed
Apr 11, 2024
Contributor
Vectorized
left a comment
Vectorized
left a comment
There was a problem hiding this comment.
I think ERC6551AccountUpgradeable should also be updated with the hardfork support too.
…ded hardfork compatibility to upgradeable example
Contributor
Author
|
@Vectorized good catch, the logic here was a bit wonky. Was trying to ensure that accounts deployed on chains other than the NFT's home chain will still return a null owner post-hardfork. This can be accomplished in a simpler way by checking against Added a test case to show the expected behavior. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.