Repository navigation
fix: count single token ERC721 approvals, keep contract wallet signatures, reject zero unitsToFill, pass extraData to the zone - #1058
Merged
Conversation
approvedItemAmount only read isApprovedForAll, so the approve(operator, id) that exactApproval itself sends was never counted. A listing created with exactApproval was then rejected by fulfillOrder (basic and standard route) and fulfillOrders with "The offerer does not have the sufficient approvals.", although Seaport fills it onchain, and every later action on that token asked for the same approval again. When the operator is not approved for all, check getApproved for an ERC721 item, using the resolved identifier for a criteria item as balanceOf does.
signOrder and signBulkOrder turned every 65 byte signature into its EIP-2098 compact form, which is only safe for the offerer's own ECDSA signature. A contract wallet such as a 1-of-1 Safe also returns 65 bytes, Seaport passes it to the wallet's isValidSignature as given, and the wallet rejects the 64 byte rewrite. A Safe eth_sign signature (v 31 or 32) made ethers throw instead. Compact only when the signature recovers to the offerer.
The fill paths treat a falsy unitsToFill as "not set" and fill what is left of the order, so 0 or 0n bought the entire order. The same zero as the string "0" already threw "Units to fill must be greater than 0" in mapOrderAmountsFromUnitsToFill. Check it once in fulfillOrder and fulfillOrders so every zero is rejected the same way.
fulfillOrder takes an extraData argument, but only fulfillAdvancedOrder can carry it. When the order qualified for the basic route, or for plain fulfillOrder (no units to fill, no criteria items, no gift recipient), the SDK picked that route and the caller's extraData never reached Seaport. Both of those entry points hand the zone an empty extraData. For an open order that goes unnoticed, but a restricted order whose zone reads extraData, such as a signed zone that expects its signature there, reverts in authorizeOrder even though the caller supplied the right bytes. Treat non-empty extraData like the other inputs that need the advanced route: skip the basic route and call fulfillAdvancedOrder. Fills without extraData are unchanged. fulfillOrders already submits through fulfillAvailableAdvancedOrders and was not affected. The new test deploys a zone that rejects empty extraData and records what it receives, and fulfills a restricted single ERC721 listing (the basic route) and a restricted two item bundle (the fulfillOrder route) with extraData. Both reverted with MissingExtraData before this change.
This was referenced Oct 30, 2026
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.
Motivation
Five community PRs opened this week. Three of them touch
src/seaport.tsand three touchsrc/utils/fulfill.ts, so they are stacked here with each contributor's commit kept as authored. All five cherry-picked ontomainwithout conflicts, and no follow-up commits were needed.Fixes #1052 and #1055. Supersedes #1051, #1053, #1054, #1056 and #1057, which need closing once this merges.
Solution
Commits, in merge order (isolated files first, shared files last):
approvedItemAmountread onlyisApprovedForAll, so an ERC721 approved one token at a time (which is whatexactApproval: truedoes) counted as unapproved. A listing created that way was rejected byfulfillOrderandfulfillOrderswith "The offerer does not have the sufficient approvals." even though Seaport fills it onchain. Now readsgetApproved(id)for ERC721 items when the operator is not approved for all, using the resolved criteria identifier where there is one.orderIndex-itemIndexdirectly after the token id, so id1in order11and id11in order1produced the same key and were grouped into one fulfillment. Adds a-separator at both sites (offer and consideration).signOrderandsignBulkOrdercompacted every 65 byte signature to EIP-2098 form. A contract wallet can also return 65 bytes, and Seaport hands the bytes toisValidSignatureunchanged when ecrecover does not yield the offerer, so the wallet received bytes it never produced. Now compacts only when the signature recovers to the offerer from the signed digest. A Safe eth_sign signature with v = 31 or 32 no longer throwsinvalid v.unitsToFillof0or0nis falsy and was read as "not set", which fills the whole remaining order, while"0"threw. Now any setunitsToFillthat is not greater than zero throws at the top offulfillOrderand for each entry infulfillOrders.fulfillOrder, a caller'sextraDatawas dropped, because onlyfulfillAdvancedOrdercarries it. A restricted order whose zone readsextraDatathen reverted. Non-emptyextraDatanow forces the advanced route at both route decisions.Other call sites, checked for each fix:
approvedItemAmounthas one caller,getBalancesAndApprovals, which is now passed the criteria. Lookups downstream are keyed by token and identifier, so a per-token1nis matched to its own identifier.signOrderandsignBulkOrder, both changed.unitsToFillenters throughfulfillOrderandfulfillOrdersonly, both guarded.shouldUseBasicFulfillgate inseaport.tsanduseAdvancedinfulfillStandardOrder, both changed.fulfillOrdersalready usesfulfillAvailableAdvancedOrdersand passesextraData.Reachability from opensea-sdk, which depends on
@opensea/seaport-js: it callscreateOrderandcreateBulkOrders(so #1053 affects its contract wallet users, and #1056 changes whether an approval action is returned when a token was already approved individually). It does not callfulfillOrderorfulfillOrders, so #1051, #1054 and #1057 only affect direct seaport-js callers.Impact
fulfillOrderscall, from the same offerer and collection, with ids that concatenate with an order index identically. What Seaport does with the merged group was not measured onchain, by the contributor or here.main,fulfillOrderwithunitsToFill: 0nreturns actions instead of rejecting (reproduced). The contributor's measurement that the fulfiller then receives all 10 units of the listing was not repeated here.TestERC1271WrappedWallet, a test wallet that verifies the way a 1-of-1 Safe does, not against a deployed Safe.Tests
On this branch:
npm run lint(tsc plus Biome) exit 0,npm test299 passing,npm run coverageexit 0 (all files 98.72% lines,approval.ts100% lines).Each test was rerun with only its source hunk reverted on this branch, and failed on its own assertion:
approval.ts,balanceAndApprovalCheck.ts(#1056)execute-approvals.spec.tsError: The offerer does not have the sufficient approvals.(3 fill cases),expected [ 'approval', 'create' ] to deeply equal [ 'create' ],expected [ 'approval', 'exchange' ] to deeply equal [ 'exchange' ]fulfill.tskey hunk (#1051)aggregate-key-collision.spec.tsexpected [ [ { orderIndex: 1, ...}, ... ] ] to deeply equal [ [ { orderIndex: 1, ... } ], ... ]seaport.tscompaction (#1053)sign-order.spec.ts, 1 increate-order.spec.tsexpected 64 to equal 65,TypeError: invalid v (argument="v", value=31 ...),expected 99 to equal 100. With the length assertion also removed, the fill reverts with'signature must be 65 bytes'seaport.tsguards (#1054), and thefulfillOrdersguard alonepartial-fulfill.spec.tsexpected promise to be rejected with an error including 'Units to fill must be greater than 0' but it was fulfilled with { actions: [ ... ] }seaport.tsroute gate alone (#1057)fulfill-extra-data.spec.tsreverted with custom error 'MissingExtraData()'fulfill.tsuseAdvancedalone (#1057)fulfill-extra-data.spec.tsreverted with custom error 'MissingExtraData()'Removed or moved guards
None. Every change adds a condition. Diffed each changed function against
main: #1056 replaces a single return with the sameMAX_INTreturn plus a fallback that can only turn0ninto1n; #1053 keeps thelength === 132check and adds conditions in front of compaction; #1057 adds a term to two route conditions.Release
#1054 adds a throw on input that previously returned, so the next release is a minor: 4.5.0. No version bump is in this PR.
Notes for review
getApprovedcall per ERC721 item when the operator is not approved for all. For a token that does not exist,getApprovedreverts, butbalanceOfin the samePromise.allalready callsownerOf, which reverts too, so there is no new failure mode there. A contract that implementsisApprovedForAllbut notgetApprovedwould now throw where it used to return an approval action. ERC-721 requires both, so this was left as is.src/andtest/, no new dependencies, no workflow or script changes).Confidence: high. Each fix is small, has a tripwire test, and the stack passes lint, tests and coverage.
🤖 Generated with Claude Code