Repository navigation
fix: drop expired orders from a batch fulfill instead of failing it - #1063
Merged
ryanio merged 1 commit intoNov 6, 2026
Merged
Conversation
fulfillOrders builds a fulfillAvailableAdvancedOrders call, and Seaport skips any order in it whose endTime has passed, the same way it skips a cancelled or filled one. The SDK already drops cancelled and filled orders before building the call, but it kept expired ones. Each expired order was still held to its offerer's balance and approvals, still added to the fulfiller's cumulative ERC20 check, and still added to the native value sent. So a single lapsed order could block a batch the contract would settle: * an expired listing whose seller has since moved the NFT made fulfillOrders throw "The offerer does not have the amount needed to create or fulfill.", while the contract returns availableOrders [false, true] for the same two orders and fills the live one; * a fulfiller holding enough ERC20 for the live orders, but not for the live and expired ones together, got "The fulfiller does not have the balances needed to fulfill." Drop an order whose endTime is at or before the latest block timestamp, next to the cancelled and filled ones. Block time only moves forward, so such an order is still expired when the transaction lands. Orders that have not started yet are kept, since they can become active by then.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1063 +/- ##
==========================================
+ Coverage 98.27% 99.17% +0.89%
==========================================
Files 35 60 +25
Lines 14526 22200 +7674
Branches 660 1284 +624
==========================================
+ Hits 14276 22017 +7741
+ Misses 245 176 -69
- Partials 5 7 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
|
Thanks for this. I checked it against current main: both integration tests fail without the change, with the errors you described, and pass with it. Lint, tests and coverage are green. Merged, and it will go out in the next release. |
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
fulfillOrdersis documented as best effort, and #985 made it drop cancelled and fully filled orders so one stale order does not take the whole batch down. Expired orders were left in.Seaport treats an expired order the same way as a cancelled one in
fulfillAvailableAdvancedOrders: it skips it and settles the rest. The SDK still builds the call with the expired order inside, so that order is:valuesent.That makes one lapsed order enough to block a batch the contract would settle. Two cases, both reproduced on the hardhat fixture at 589c6b8:
fulfillOrdersthrowsThe offerer does not have the amount needed to create or fulfill.CallingfulfillAvailableAdvancedOrderson the Seaport contract with the same two orders returnsavailableOrders [false, true]and transfers the live listing's NFT for exactly its price.fulfillOrdersthrowsThe fulfiller does not have the balances needed to fulfill.although only the live order would be charged.Expiry is the most common way an order goes stale between reading it and filling it, which is the case this method exists for.
Solution
Drop an order from the batch when its
endTimeis at or before the latest block timestamp, alongside the cancelled and filled ones (hasOrderExpirednext toisOrderFulfillable). Block time only moves forward, so an order that has expired at the latest block is still expired when the transaction lands, which matches Seaport'sstartTime <= block.timestamp < endTimewindow. Orders that have not started yet are kept, because they can become active by the time the transaction is mined.If every order is dropped the existing "None of the orders can be fulfilled" error is thrown, now naming expiry too, instead of sending a transaction that reverts with
NoSpecifiedOrdersAvailable().Single order
fulfillOrderis unchanged.Tests:
fulfill-orders-availability.spec.ts: an expired listing whose NFT the seller moved no longer blocks the live listing, and the value sent is only the live listing's price; an expired ERC20 listing is no longer counted against the fulfiller's balance, and the live one settles on chain with the fulfiller's exact balance.fulfill-utils.spec.ts:hasOrderExpiredboundaries (before, at and afterendTime, and a not yet started order).Both new integration tests fail on
mainwith the errors above and pass with the change. Full suite: 305 passing,tsc --noEmitclean,biome check .clean apart from the existing config migration info.