sudoswap improved price calculations

[0xRobin]

following the discussion we had after the merge of: #1397
This PR includes:

• improved ETH tracing to determine the correct trade amount
• aggregator fields population
• test with example trades

For Dune Engine V2
I've checked that:

• I tested the query on dune.com after compiling the model with dbt compile (compiled queries are written to the target directory)
• I used "refs" to reference other models in this repo and "sources" to reference raw or decoded tables
• if adding a new model, I added a test
• the filename is unique and ends with .sql
• each sql file is a select statement and has only one view, table or function defined
• column names are lowercase_snake_cased

When you are ready for a review, tag duneanalytics/data-experience. We will re-open your forked pull request as an internal pull request. Then your spells will run in dbt and the logs will be avaiable in Github Actions DBT Slim CI. This job will only run the models and tests changed by your PR compared to the production project.

[0xRobin]

@andrewhong5297 give this a look if you can.
I haven't finished checking everything but query seems to run.
The big change is to acknowledge that the trade price we should report is the amount the buyer payed for the NFTs.

I want to gather some example transactions covering all the different trade cases and write a test for them.
Seems like the most optimal way to verify the calcs are correct.

[andrewhong5297]

Awesome!!! I'll take a look tonight, agree on having tests for different trade types.

Probably need to have pool type checks with trade types, and add in the total protocol fee check as a sanity check.

Likely also need an aggregator check (I saw Vasa tweet that gem was integrating next week).

[0xRobin]

0xRob's Main nft.trades framework

(this is not sudo specific)

When reporting trades and determine the actual value exchanged I like to think of states and state transformations.
A basic NFT trade can be distilled to these parties:

B) Buyer, S) Seller F) Facilitators (marketplaces/protocols/aggregators/...)

flowchart LR;
    B(Buyer) -->|ETH| S(Seller);
    S(Seller) -->|NFT| B(Buyer);
    B(Buyer) -.->|ETH| F(Facilitators);

Loading

These are the state transformations for each:

stateDiagram-v2
    b0 : B0
    b1 : B1
    s0 : S0
    s1 : S1
    f0 : F0
    f1 : F1
    b0 --> b1 : +1 NFT <br> - ΔB ETH
    s0 --> s1 : -1 NFT <br> + ΔS ETH
    f0 --> f1 : + ΔF ETH

Loading

Where in general it must hold that:
$\Delta_B = \Delta_S + \sum_{i}{\Delta_{F_i}}$

In short:

• The Buyer pays what the Seller and all intermediate Facilitators receive (fees)
• The trade value is what the Buyer pays

---

sudoswap

If we apply this to sudo, it becomes clear that we can correctly determine:

• the trade value by tracing down what the buyer paid.
• the fee value by tracing down what the protocol received

Which is what we are doing here:

spellbook/models/sudoswap/ethereum/sudoswap_ethereum_events.sql

Lines 174 to 187 in de1d028

	, SUM(
	CASE WHEN sb.trade_category = 'Buy' -- caller buys, AMM sells
	THEN (
	CASE WHEN tr.from = sb.call_from THEN value -- amount of ETH payed
	WHEN (tr.to = sb.call_from AND sb.call_from != sb.asset_recip) THEN -value --refunds unless the caller is also the asset recipient, no way to discriminate there.
	ELSE 0 END)
	ELSE ( -- caller sells, AMM buys
	CASE WHEN tr.from = sb.pair_address THEN value -- all ETH leaving the pool, nothing should be coming in on a sell.
	ELSE 0 END)
	END ) as trade_price -- what the buyer paid (incl all fees)
	, SUM(
	CASE WHEN (tr.to = sb.protocolfee_recipient) THEN value
	ELSE 0 END
	) as protocol_fee_amount -- what the buyer paid

What we are not seeing here is the so-called swap fee set by a trade pool owner (owner fee).
I would say the swap fee has no influence in the value exchanged, it only influences the spread when setting both ask and bid prices on the same moving price curve. When you are listing NFTs at 10% above the current floor price, you are not collecting a 10% fee when that ask gets filled.

You should look at it as a spread setting instead of a fee setting where it results in profit if (and only if) spread from both the ask and bid sides are captured.
Now that I have thought about it more, I believe we should remove the owner fee fields from this base table as it will probably lead to a lot of misinformation about pool profits.
It's still interesting to calculate the 'virtual swap fees' that were used in all the pool trades, but more thought is needed to present it correctly to the public then just a flat fee collected on each swap.

I hope this gives a good/better understanding at the thought framework I'm currently employing. 😄

[andrewhong5297]

wait we still need owner fees in there! I thought we agreed to keep all fees in prices and those columns too. I can't accept deleting those columns completely.

[andrewhong5297]

if you take those out there's no way for us to back into real spot price either if we're doing slippage analysis later

[0xRobin]

if you take those out there's no way for us to back into real spot price either if we're doing slippage analysis later

You can still calculate internal spot price as protocol_fee_amount/protocol_fee_percentage

[andrewhong5297]

if you take those out there's no way for us to back into real spot price either if we're doing slippage analysis later

You can still calculate internal spot price as protocol_fee_amount/protocol_fee_percentage

lol fair, but I am still against taking it out since I think it's useful data to have.

[0xRobin]

I think it's useful data to have.

will revert then, no harm in including it. 👍

[0xRobin]

@jeff-dude this is now ready for review :)

[jeff-dude]

@0xRobin i pushed a few minor commits. the model runs as expected. i did compile the code and grab the test query on new seed file, it appears rows are returned:
https://dune.com/queries/1221898

[0xRobin]

We actually want to do outer join here because we want the the union of the two sets.
To validate that we are

1. not missing any entries
2. not returning excess entries

[jeff-dude]

got it, i think syntax needs to be 'full outer join' or it fails, i can push a commit if so

[0xRobin]

@0xRobin i pushed a few minor commits. the model runs as expected. i did compile the code and grab the test query on new seed file, it appears rows are returned:

@jeff-dude thanks for this!
I manually checked the data and the issue was actually with the seed file where there were often not enough decimals reported. Should be resolved now, can you revalidate the test?

[jeff-dude]

@0xRobin i pushed a few minor commits. the model runs as expected. i did compile the code and grab the test query on new seed file, it appears rows are returned:

@jeff-dude thanks for this! I manually checked the data and the issue was actually with the seed file where there were often not enough decimals reported. Should be resolved now, can you revalidate the test?

rebuilding now

[jeff-dude]

thank you @0xRobin and @andrewhong5297

fyi @soispoke this model will need a full refresh in prod