chainflip-io / chainflip-backend

The Chainflip backend repo, including the Chainflip Node and CFE.

48 stars 15 forks source link

Feat: Solana gas tweaks #5140

Closed albert-llimos closed 1 month ago

albert-llimos commented 1 month ago

Pull Request

Closes: PRO-1501

Checklist

Please conduct a thorough self-review before opening the PR.

[x] I am confident that the code works.
[ ] I have written sufficient tests.
[ ] I have written and tested required migrations.
[x] I have updated documentation where appropriate.

Summary

Set maximum CU per CCM transfer to 600k instead of 1.4M to avoid huge transactions which are less likely to be included by the leaders.
Set a minimum of 1 priority fee (1 microlamport / CU), even if the median reported fee is zero. The cost of that priority fee will be negligible while it increases significantly the chances of being included.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71%. Comparing base (f3876fa) to head (471bdfd). Report is 3 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #5140 +/- ## ====================================== - Coverage 71% 71% -0% ====================================== Files 460 460 Lines 81799 81768 -31 Branches 81799 81768 -31 ====================================== - Hits 58315 58214 -101 - Misses 20391 20458 +67 - Partials 3093 3096 +3 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

syan095 commented 1 month ago

Sorry for hijacking the PR. I've renamed calculate_ccm_gas_limit function so it's clearer that it's only used for CCM functions. (renaming only, no logic change)

syan095 commented 1 month ago

1 question - we set the minimum "priority" fee to 1 - should we set this a bit higher?

albert-llimos commented 1 month ago

1 question - we set the minimum "priority" fee to 1 - should we set this a bit higher?

While setting it to one (microlamports/CU) looks and is very low, in normal conditions (not high congestion) of the network a priority fee of 1 should be enough. However, since we're really interested in inclusion and that amount is still very small it's reasonable to raise it to 10 for now. I'll do that.