Adjust Tips Correctly for Partial Fills

[sofianeOuafir]

Motivation

The Seaport SDK's tipping functionality does not correctly adjust tip amounts in scenarios involving partial fills of orders. This led to discrepancies in the expected vs. actual tip amounts being asked to the order fulfiller. I believe that the totalNativeAmount variable calculated by the getSummedTokenAndIdentifierAmounts function needs to receive an items array that has tips adjusted (taking into account unitsToFill and totalSize). Currently, in the context of a partial fill, the SDK correctly adjusts all considerations except for the tips when calculating the totalNativeAmount variable.

This issue could impact user experience where a fulfiller would overpay upfront and then be refunded by the seaport protocol. The need for accurate and predictable transaction handling motivated this fix. This PR aims to solve this issue by correctly adjusting tips based on the portion of the order that's filled.

Scenario Description:

An NFT seller creates an order to sell 10 editions of a unique ERC1155 NFT, each priced at 1 ETH. To support a community initiative, they also include a tip as part of the transaction.

Initial Order Setup:

NFT Offer: 10 editions of a digital artwork (ERC1155 token).
Sale Price: 1 ETH per edition (total 10 ETH for all editions).
Tip: 1 ETH to a community wallet.

Expected Behavior in Partial Fulfillment:

A buyer decides to purchase 3 out of the 10 editions. The expected cost should be 3 ETH for the NFTs and a proportionally adjusted tip amount. Ideally, the tip should be adjusted to 0.3 ETH (since 3/10 of the total offer is being fulfilled).

Bug Manifestation:

Under the current SDK functionality, when the buyer completes this partial purchase, the total charged amount is not what they expect. The SDK incorrectly processes the tip without adjustment:
Actual Charge: 4 ETH (3 ETH for NFTs + 1 ETH unadjusted tip).
Expected Charge: 3.3 ETH (3 ETH for NFTs + 0.3 ETH adjusted tip).

Notes:

• Buyers initially get charged 4 ETH and the on-chain logic charges the right amount. the seaport protocols correctly adjust the tip but the seaport SDK doesn't: it sends the 0.3 ETH tip and refunds the 0.7 ETH to the transaction sender.
• I've tried to pass a tip amount that's already adjusted when calling seaport.fulfillOrder. However, it doesn't work since the seaport protocol would adjust the adjusted tip when processing the transaction on-chain (resulting in an incorrect tip amount sent to the tip receiver).

Solution

The solution involves creating and calling an adjustTipsForPartialFills function to accurately adjust tip amounts based on the filled portion of an order. The changes include:

1. Implementing logic within the adjustTipsForPartialFills function to correctly calculate the adjusted tip amounts.
2. Integrating this function within the order fulfillment process, specifically in the fulfillStandardOrder function, to apply these adjustments.

With these changes, the Seaport SDK would correctly calculate the totalNativeAmount value and correctly handle tip amounts in partial fill scenarios, ensuring accurate and reliable processing of these transactions.

[ryanio]

thank you for this PR @sofianeOuafir! do you think you could add a couple tests to verify the functionality is as expected?

[sofianeOuafir]

thank you for this PR @sofianeOuafir! do you think you could add a couple tests to verify the functionality is as expected?

Sounds good, I'll push a commit over the weekend :)

[ryanio]

@sofianeOuafir great, thanks so much! just FYI i have an open PR to update to ethers v6 replacing bignumber with bigint, i hope to finish today and if so if you can rebase and update based on that when you start over the weekend would be excellent. thanks again.

[sofianeOuafir]

@ryanio just an update: I didn't have a chance last weekend but wrapping up this PR is on my todo list this week 👍

[sofianeOuafir]

Hey @ryanio, I've added a test and rebased. Also, I've updated my code to be compatible with ethers v6 - swapped out BigNumber for bigint and made all the necessary tweaks. Let me know if there's anything else you need on this. Thanks!

[ryanio]

@sofianeOuafir thanks so much for following up! will try to get this reviewed and merged in today, then can do a release

[sofianeOuafir]

@sofianeOuafir thanks so much for following up! will try to get this reviewed and merged in today, then can do a release

🎉 thank you so much for the approval. And it's my opportunity to say a big thank you to the team at Opensea for the seaport protocol & javascript SDK ❤️

[ryanio]

hey @sofianeOuafir! of course no problem thank you for contributing back to open source.

i talked with my colleague and in particular we'd like to see 2 more tests added to confirm the right behavior:

Test 1
A case where the scaling down is really low denominations, particularly a prime number
e.g.:
buying an NFT that only costs 10 wei, partial order, only want to buy 1 out of 2 NFTs
and tip 1 wei each for 12 wei total
so it should be scaled down to 5 and 1 instead of 10 and 2

Test 2
Would be nice to test tae non-eth case, e.g. with WETH does the right amount leave your wallet and paid properly

with these 2 added, we can have more confidence of the behavior and get this included in the next version release!

thanks again,
ryan

[sofianeOuafir]

hey @sofianeOuafir! of course no problem thank you for contributing back to open source.

i talked with my colleague and in particular we'd like to see 2 more tests added to confirm the right behavior:

Test 1 A case where the scaling down is really low denominations, particularly a prime number e.g.: buying an NFT that only costs 10 wei, partial order, only want to buy 1 out of 2 NFTs and tip 1 wei each for 12 wei total so it should be scaled down to 5 and 1 instead of 10 and 2

Test 2 Would be nice to test tae non-eth case, e.g. with WETH does the right amount leave your wallet and paid properly

with these 2 added, we can have more confidence of the behavior and get this included in the next version release!

thanks again, ryan

of course, I'll add those tests and get back to you then 👍

[sofianeOuafir]

hey @ryanio, I've added the latest tests

[ryanio]

thank you!! will review and hope to get in merged in

[ryanio]

@sofianeOuafir thanks for your contribution! have released as v3.0.1