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

feat: accurate gas calculation for Amplifier API reporting #18

Merged
merged 35 commits into from
Jan 22, 2025

Conversation

roberts-pumpurs
Copy link
Member

@roberts-pumpurs roberts-pumpurs commented Jan 20, 2025

Closes #15

Computes the total gas cost (in lamports) for a given Solana transaction, factoring in:

  • Direct transaction cost.

  • Additional costs from multi-step gateway instructions (e.g., verification sessions, message payload uploads).

  • added unittests to ensure that we parse & compute gas for message approval and message execution

  • Implemented a function that will scrape the Solana RPCs to build an accurate model of how much was paid in fees for every action

@roberts-pumpurs roberts-pumpurs self-assigned this Jan 21, 2025
@roberts-pumpurs roberts-pumpurs force-pushed the rob/accurate-gas-calculation branch from 2f3861b to c8edf3e Compare January 21, 2025 12:47
@roberts-pumpurs roberts-pumpurs force-pushed the rob/accurate-gas-calculation branch from 2511cd4 to 452159f Compare January 21, 2025 13:25
Copy link
Member

@eloylp eloylp left a comment

Choose a reason for hiding this comment

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

It overall looks good to me 👍 but take my review with pinct of salt, as i am not fullly familiar with the code yet. I would just fix the CI and get rid of the unwraps (as you already noted).

Maybe in a future we could save some lines of code by improving local test helpers. Not on this PR though, as i think we are still discovering the patterns.

Just sharing one random though that came to my mind was if adding a positive margin to the gas sum (I saw it in other projects) would be needed here ? But probably that's not our job here, as this is only part of the calculation (The Solana side). Such a margin should be added by the last aggregation layer if that's the case.

crates/solana-event-forwarder/src/component.rs Outdated Show resolved Hide resolved
@roberts-pumpurs roberts-pumpurs force-pushed the rob/accurate-gas-calculation branch 2 times, most recently from 036b7a8 to cf1c030 Compare January 21, 2025 18:46
@roberts-pumpurs roberts-pumpurs force-pushed the rob/accurate-gas-calculation branch from cf1c030 to 9aeeed6 Compare January 21, 2025 18:53
frenzox
frenzox previously approved these changes Jan 22, 2025
Copy link
Contributor

@frenzox frenzox left a comment

Choose a reason for hiding this comment

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

lgtm!

@roberts-pumpurs roberts-pumpurs merged commit d704871 into main Jan 22, 2025
6 checks passed
@roberts-pumpurs roberts-pumpurs deleted the rob/accurate-gas-calculation branch January 22, 2025 13:33
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.

Relayer: recalculate accurate total_cost for tx
3 participants