fix: batch NFT ownership checks via Multicall3#8281
Open
Conversation
7 tasks
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
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.
Explanation
AssetsContractControllermessenger calls (getERC721OwnerOf,getERC1155BalanceOf) with a newgetNftOwnershipForMultipleNftsutility that batches ERC-721ownerOfand ERC-1155balanceOfcalls through Multicall3'saggregate3, falling back to individual RPC calls on unsupported chains or when multicall fails.checkAndUpdateSingleNftOwnershipStatusin favor ofcheckAndUpdateAllNftsOwnershipStatus, which now batches all NFTs in a single pass.standardparameter toisNftOwnerso callers that already know the token standard skip redundant subcalls.Breaking Changes
checkAndUpdateSingleNftOwnershipStatusremoved — usecheckAndUpdateAllNftsOwnershipStatusinstead.AllowedActionsnarrowed —AssetsContractController:getERC721OwnerOfandAssetsContractController:getERC1155BalanceOfare no longer required byNftController's messenger. Consumers constructing the messenger must remove these from their allowed actions list.References
https://consensyssoftware.atlassian.net/browse/ASSETS-2959
Checklist
Note
Medium Risk
Changes NFT ownership verification logic and introduces new batching/fallback behavior, plus removes a public
NftControllermethod and required messenger actions (breaking for integrators). Risk is mitigated by extensive new unit coverage but could affect ownership status correctness on edge-case contracts/chains.Overview
NFT ownership checks are refactored to batch calls via Multicall3.
NftController.isNftOwnerandcheckAndUpdateAllNftsOwnershipStatusnow use a newgetNftOwnershipForMultipleNftshelper that batches ERC-721ownerOf/ ERC-1155balanceOfthrough Multicall3, with fallback to capped individual RPC calls when Multicall isn’t available or fails.Breaking API/messenger changes:
NftControllerno longer requiresAssetsContractController:getERC721OwnerOforAssetsContractController:getERC1155BalanceOf(removed fromAllowedActions), andcheckAndUpdateSingleNftOwnershipStatusis removed in favor of the batchedcheckAndUpdateAllNftsOwnershipStatusflow. Tests are updated and expanded to cover multicall/individual paths, standard-hinted subcall reduction, and failure/inconclusive-result behavior.Written by Cursor Bugbot for commit 7b4faee. This will update automatically on new commits. Configure here.