Skip to content

1.3#984

Merged
0age merged 36 commits intomainfrom
1.3
Feb 14, 2023
Merged

1.3#984
0age merged 36 commits intomainfrom
1.3

Conversation

@0age
Copy link
Contributor

@0age 0age commented Feb 14, 2023

Seaport version 1.3 addresses two limitations inherent to version 1.2:

  • On 1.2, token transfers could occur after restricted and contract order checks (validateOrder and ratifyOrder) under specific conditions (namely, when multiple orders are being fulfilled at once and conduits are utilized or there are unspent offer item amounts). 1.3 ensures that all token transfers across all fulfilled orders, even returning excess native tokens, occur before any restricted or contract order checks are performed. (Be sure not to transfer any tokens back to Seaport as part of zone or contract offerer logic as they will not be recoverable during the current fulfillment.) This change will make post-execution zone and contract offerers much easier to work with, as otherwise they would need to perform redundant checks and would require the fulfiller to specifically tailor their fulfillment methods to work around the transfer limitations — huge thanks to @androolloyd for raising this issue to the Seaport Working Group!
  • On 1.2, any fulfillments where the offerer and the recipient have the same address would be "filtered" from the executions array (as there is no reason an associated transfer needs to be performed). 1.3 allows for contract offerers to offer native tokens which are expected to be provided to Seaport as part of the call to generateOrder — in these cases, or as part of a match operation, those native tokens have already been supplied to Seaport by the time the execution is filtered, meaning the corresponding consideration item would not be accounted for and those native tokens would eventually be returned to the caller instead. (This means that offerers on 1.2 need to be very careful not to simultaneously offer native tokens and require native tokens as consideration items; this mainly affects contract offerers.) In 1.3, executions with native token item types are not filtered from the executions array.

There are also some additional optimizations:

  • The typehash directory has been replaced with a large conditional statement that selects an appropriate constant typehash value based on the height of the bulk order tree. This avoids needing to perform an extcodecopy (thereby touching another account and costing 2600+ gas) at the expense of additional runtime code on Seaport itself.
  • Some additional performance improvements have been made as a consequence of addressing the limitations above; for instance, fulfilling multiple orders where no restricted or contract orders are present is now less expensive, and implementing the _unmaskedAddressComparison check inline with the native token check as part of execution filtering led to the removal of a number of superfluous mask operations (it turns out that function was actually not unmasked as the address arguments were still being masked when entering the function).

Finally, the version has been bumped from 1.2 to 1.3. There are additional changes required to bring the reference contracts fully in-line with the optimized contracts, and more tests to be written to validate the new behavior and ensure that coverage gaps with implementing zones and contract offerers are fully addressed, but (barring any unforeseen issues) this PR contains the full set of changes between Seaport 1.2 and 1.3.

0age and others added 30 commits February 13, 2023 14:41
loop through orders again if restricted/contract present
…executions

do not filter native token executions
…n-ben/ZoneContractOrderValidator

linting, cleanup, and a tiny refactor
…before-checks

move excess native token transfer to before post-execution checks
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2023

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (6e6d006) compared to base (e99e1e3).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #984   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           47        47           
  Lines         2661      2671   +10     
  Branches       393       394    +1     
=========================================
+ Hits          2661      2671   +10     
Flag Coverage Δ
foundry 77.71% <81.25%> (-0.10%) ⬇️
production 100.00% <100.00%> (ø)
reference 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
contracts/Seaport.sol 100.00% <ø> (ø)
contracts/lib/Consideration.sol 100.00% <ø> (ø)
contracts/lib/ConsiderationDecoder.sol 100.00% <ø> (ø)
contracts/lib/LowLevelHelpers.sol 100.00% <ø> (ø)
reference/ReferenceConsideration.sol 100.00% <ø> (ø)
contracts/lib/ConsiderationBase.sol 100.00% <100.00%> (ø)
contracts/lib/FulfillmentApplier.sol 100.00% <100.00%> (ø)
contracts/lib/OrderCombiner.sol 100.00% <100.00%> (ø)
contracts/lib/Verifiers.sol 100.00% <100.00%> (ø)
reference/lib/ReferenceConsiderationBase.sol 100.00% <100.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

…ost-execution

TestTransferValidationZoneOfferer fulfillAvailable tests with conduit + Add sol helpers
@0age 0age merged commit 718670a into main Feb 14, 2023
@0age 0age deleted the 1.3 branch February 14, 2023 22:43
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.

4 participants