Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] wrong fee amount for phoenix_v1.base_trades #6642

Open
0xroll opened this issue Aug 31, 2024 · 4 comments
Open

[BUG] wrong fee amount for phoenix_v1.base_trades #6642

0xroll opened this issue Aug 31, 2024 · 4 comments
Assignees
Labels
bug Something isn't working dbt: dex covers the DEX dbt subproject dbt: solana covers the Solana dbt subproject in review Assignee is currently reviewing the PR

Comments

@0xroll
Copy link
Contributor

0xroll commented Aug 31, 2024

Description

The current fee_tier is overstated because the raw data is denominated in basis points (bps) but is being divided by 100 instead of 10000.

https://github.com/duneanalytics/spellbook/blob/main/dbt_subprojects/solana/models/_sector/dex/phoenix/phoenix_v1_base_trades.sql#L48

Current behavior

this is causing the inflated fees on solana_dex.trades

dune query to check :

https://dune.com/queries/4033552

Expected behavior

fee_tier to be /10000 instead of /100 to get the correct values

Impacted model(s)

https://github.com/duneanalytics/spellbook/blob/main/dbt_subprojects/solana/models/_sector/dex/phoenix/phoenix_v1_base_trades.sql

https://github.com/duneanalytics/spellbook/blob/main/dbt_subprojects/solana/models/_sector/dex/dex_solana_trades.sql

Possible solution

fix by changing from /100 to /10000

@0xroll 0xroll added the bug Something isn't working label Aug 31, 2024
@jeff-dude jeff-dude added dbt: dex covers the DEX dbt subproject dbt: solana covers the Solana dbt subproject ready-for-review this PR development is complete, please review labels Sep 3, 2024
@jeff-dude
Copy link
Member

@andrewhong5297 can you help validate the above issue on solana dex model?

@GioZaarour
Copy link

Hello, I came across this issue when writing a query here [https://dune.com/queries/4010725/6752431].

For certain markets on Phoenix, I have validated that the taker fee is in basis points. You can see this on the smart contracts on Solscan. For example, the "takerfeebps" for the SOL/USDC market is 2 bps, meaning 0.0002. See here [https://solscan.io/account/4DoNfFBfF7UokCC2FQzriy7yHK6DY6NVdYpuekQ5pRgg#data]

In the dex_solana.trades table, the fees are in percentages, for example 0.02 for the SOL/USDC market on Phoenix.

@andrewhong5297
Copy link
Collaborator

Sounds right, lets push a fix

@jeff-dude jeff-dude added in review Assignee is currently reviewing the PR and removed ready-for-review this PR development is complete, please review labels Sep 6, 2024
@GioZaarour
Copy link

Sounds right, lets push a fix

Hey Andrew! I'm not sure what happened, but from what I can see now on a new query, the amount sold/bought for the SOL token on Phoenix (not sure if this issue also occurred with other tokens on other markets), has been divided by many orders of magnitude. I'm not sure if this happened in an attempt to reduce the decimal form of the fee_tier and reduce the fee_amount, and accidentally reduced the amount of the token being swapped instead?

Regardless, you can see the error here [https://dune.com/queries/4050966]. Take a look at the amount being swapped for any trade, and then look at the amount on Solscan using the tx_id.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dbt: dex covers the DEX dbt subproject dbt: solana covers the Solana dbt subproject in review Assignee is currently reviewing the PR
Projects
None yet
Development

No branches or pull requests

4 participants