-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add Stabull fee, revenue and volume #4940
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
base: master
Are you sure you want to change the base?
Conversation
|
The stabull adapter exports: |
| options, | ||
| balances: dailyFees, | ||
| targets: [configs[options.chain].treasury], // Treasury address | ||
| fromAdddesses, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even though you are restricting the source here, it still has room for bugs and a bit inefficient as we are pulling two sets of logs when trade log already has fee info, also what happens when they change the treasury address?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the feeData on the event is raw and needs some calculations based on Oracle data at that exact moment to figure out the exact fee amount , unfortunately there is no event to read the calculated amount.
here is the respective code:
https://basescan.org/address/0x1246B19c59FfF6A92E875b57402743cf576C86bB#code#L2184
for the treasury address i can dynamically read it from curve contracts
https://basescan.org/address/0x86Ba17ebf8819f7fd32Cf1A43AbCaAe541A5BEbf#readContract#F10
if you still think this way of calculation of fee/revenue is inefficient, i can discard all of these and calculate fees based on volume. since the rates are fixed i think it will achieve the same result
| }) | ||
|
|
||
| for (const event of tradeEvents) { | ||
| dailyVolume.addToken(event.origin, event.originAmount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you use the addOneToken here, not strictly necessary, what it does is, if one of the token is well known like eth or a stablecoin, uses that for fees, this way, we are more likely to have price support for the given token
| const logs = await options.getLogs({ | ||
| target: address, | ||
| eventAbi: events.newCurve, | ||
| onlyArgs: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add cacheInCloud: true here, this caches the log response
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah i was looking for a way to cache this data, didn't know about this. thanks
g1nt0ki
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
I could calculate fees and revenue based on volume, but I thought tracking the treasury address might be a better approach, in case they add new sources of revenue in the future.