Skip to content

fix: pass fulfillOrder extraData to the zone instead of dropping it - #1057

Closed
erkancamli wants to merge 1 commit into
ProjectOpenSea:mainfrom
erkancamli:fix/fulfill-order-extra-data
Closed

erkancamli wants to merge 1 commit into
ProjectOpenSea:mainfrom
erkancamli:fix/fulfill-order-extra-data

Conversation

@erkancamli

Copy link
Copy Markdown
Contributor

Motivation

fulfillOrder accepts extraData, but only fulfillAdvancedOrder can carry it to Seaport. When an order qualifies for the basic route, or for plain fulfillOrder (no unitsToFill, no criteria items, no gift recipient), the SDK takes that route and the caller's extraData is silently dropped. Seaport then calls the zone with empty extraData on both of those entry points: the basic route builds the zone call in _assertRestrictedBasicOrderAuthorization, and fulfillOrder decodes the Order as an AdvancedOrder whose extraData is empty bytes.

For an open order nothing notices, but a restricted order whose zone reads extraData, for example a signed zone that expects its signature there, reverts in authorizeOrder even though the caller passed the right bytes. The most common case is the hottest one: a restricted single ERC721 listing for ETH, which always goes through the basic route.

Solution

Treat non empty extraData like the other inputs that already force the advanced route:

  • fulfillOrder skips the basic route when extraData is non empty.
  • fulfillStandardOrder uses fulfillAdvancedOrder when extraData is non empty.

A small hasExtraData helper decides this, so "0x", "" and undefined keep today's behavior and fills without extraData still use the cheaper routes. fulfillOrders already goes through fulfillAvailableAdvancedOrders and was not affected.

Tests: test/fulfill-extra-data.spec.ts deploys a small TestExtraDataZone that rejects empty extraData and records what it receives, then fulfills a restricted single ERC721 listing (basic route) and a restricted two item bundle (fulfillOrder route) with extraData. Both reverted with MissingExtraData() before this change and pass after it. hasExtraData also gets unit coverage in test/fulfill-utils.spec.ts.

npm test: 283 passing. npm run check-types and biome check . are clean.

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.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.14%. Comparing base (f618d19) to head (4e290ca).
⚠️ Report is 425 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1057      +/-   ##
==========================================
+ Coverage   98.27%   99.14%   +0.86%     
==========================================
  Files          35       58      +23     
  Lines       14526    21323    +6797     
  Branches      660     1210     +550     
==========================================
+ Hits        14276    21141    +6865     
+ Misses        245      175      -70     
- Partials        5        7       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ryanio

ryanio commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Thanks @erkancamli. Confirmed: reverting the basic-route gate fails the single ERC721 case with MissingExtraData(), and reverting the useAdvanced change alone fails both cases. fulfillOrders already went through fulfillAvailableAdvancedOrders, as you said.

This was stacked with the other open PRs in #1058 with your commit unchanged, and it shipped in v4.5.0. Closing this one in favor of #1058.

@ryanio ryanio closed this Oct 30, 2026
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.

3 participants