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

estimate tco2 redemption amount given pool redemption amount #40

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

PawelTroka
Copy link
Contributor

@PawelTroka PawelTroka commented Jan 25, 2024

@PawelTroka PawelTroka requested a review from kosecki123 January 25, 2024 17:22
@PawelTroka PawelTroka requested a review from 0xmichalis January 25, 2024 17:51
@kosecki123
Copy link
Contributor

kosecki123 commented Jan 26, 2024

Some extra info on gas usage:

estimateTCO2RedemptionAmount gas usage depends on the max iterations amount

Higher the amount = better estimate, still even with 10 iterations we are below 1% error

40 iterations = 1267785
20 iterations = 650471
10 iterations = 341823

If we use this as view calls from outside, we should keep 40. If we want to anyways put this into redeem transaction we should go with 10 or lower.

All gas usages taken by using:

forge test --via-ir -vvvv --match-test testEstimateTCO2RedemptionAmount_NormalCase_ShouldGiveResultsWithSmallSlippage --gas-report

Copy link
Contributor

@kosecki123 kosecki123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, lets's wait for @0xmichalis

src/FeeCalculator.sol Outdated Show resolved Hide resolved
@0xmichalis
Copy link
Member

Unfortunately I am not too strong on math but I'd expect this to be implemented with the use of the inverse of the current function that estimates fees so we don't have to approximate a response. Is that wishful thinking? :) cc @aspiers

@PawelTroka
Copy link
Contributor Author

Unfortunately I am not too strong on math but I'd expect this to be implemented with the use of the inverse of the current function that estimates fees so we don't have to approximate a response. Is that wishful thinking? :) cc @aspiers

I am not sure if that is possible given we have implicit function there
redemptionAmount = poolAmount - calculateRedemptionFee(redemptionAmount, current, total)

@0xmichalis
Copy link
Member

I am not sure if that is possible given we have implicit function there
redemptionAmount = poolAmount - calculateRedemptionFee(redemptionAmount, current, total)

Hrm, I think you are right. Somehow I thought the inverse of the current curve would be useful but it doesn't make much sense on a second thought.

@aspiers
Copy link
Member

aspiers commented Jan 29, 2024

It's not an inverse function exactly. I have taken a stab at documenting here:

https://www.notion.so/toucan-protocol/Redemption-with-CHAR-spend-as-input-c88f4c4c6c7a4b1b814d8ce20681bf31

but I probably made mistakes. Can we use something more efficient than linear bisection, like Newton's method?

PawelTroka and others added 4 commits January 30, 2024 11:29
…terations in estimation algorithm can be set up after deploying the contract
…demption amount to calculate estimated pool amount
@PawelTroka PawelTroka force-pushed the feature/estimate-tco2-redemption-amount-given-pool-redemption-amount branch from 62ed793 to 9cc622a Compare January 30, 2024 10:44
@PawelTroka
Copy link
Contributor Author

It's not an inverse function exactly. I have taken a stab at documenting here:

https://www.notion.so/toucan-protocol/Redemption-with-CHAR-spend-as-input-c88f4c4c6c7a4b1b814d8ce20681bf31

but I probably made mistakes. Can we use something more efficient than linear bisection, like Newton's method?

Unfortunately I do not seem to have access to this notion page :(

To be totally honest I am not 100% sold on Newton's method as there is a small risk that it may not converge in all cases. Also a derivative for our getRedemptionFee might not be so trivial, we could use finite difference which brings us to secant method but this is no longer so superior to what we have right now while being slightly more complicated. I don't have strong opinions here but I think what we have right now is okayish for the time we spent on it.

@aspiers
Copy link
Member

aspiers commented Jan 30, 2024

@PawelTroka I gave you access to the page. (BTW I think you can maybe also request access to Notion pages - if so, feel free to do that in future!)

That's a good point about the derivative. I would have thought that given the smoothness of the curve, it should converge easily, but you're definitely a much stronger mathematician than me so I trust your judgement!

However the main question here is whether we should really be doing this estimation on-chain - please see https://github.com/neutral-protocol/dynamic-fee-pools/issues/38#issuecomment-1916932139 and https://www.notion.so/toucan-protocol/Redemption-with-CHAR-spend-as-input-c88f4c4c6c7a4b1b814d8ce20681bf31?pvs=4#e1c6949b8fa2494986a413b31c43c203

@aspiers
Copy link
Member

aspiers commented Jan 30, 2024

To be clear: if we do it on-chain atomically as part of the redemption, then the accuracy/gas trade-off is challenging and so far it's yet to be proven where there's a sweet spot in this trade-off for acceptable UX. If we do it off-chain with "slippage" protection, then accuracy and gas both cease to be issues.

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.

Support calculating the amount of TCO2 redeemed based on POOL
4 participants