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

[frontier] Add support for PoV gas refunding #3036

Merged
merged 12 commits into from
Nov 27, 2024

Conversation

Agusrodri
Copy link
Contributor

@Agusrodri Agusrodri commented Nov 8, 2024

What does it do?

Enables PoV gas refunding, which means that the difference between the benchmarked worst-case and the real proof-size consumption is now reclaimed back to the user.

⚠️ Breaking Changes ⚠️

  • The main breaking change of this PR consists in the PoV reclaim functionality itself.
  • For transactions that consume big PoV (e.g a smart contract calling many other inner smart contracts), and as long as the computed PoV by the host function is lower than the estimated one, users will be refunded with the unused PoV gas.
  • Even if the whole functionality of this PR is not expected to break anything, it's worth mentioning it as breaking, as some dapps/projects may notice these changes on their monitoring tools.

Related frontier (fork) PRs

Copy link
Contributor

github-actions bot commented Nov 8, 2024

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2248 KB (no changes) ✅

Moonbeam runtime: 2224 KB (no changes) ✅

Moonriver runtime: 2224 KB (no changes) ✅

Compared to latest release (runtime-3300)

Moonbase runtime: 2248 KB (+220 KB compared to latest release) ⚠️

Moonbeam runtime: 2224 KB (+228 KB compared to latest release) ⚠️

Moonriver runtime: 2224 KB (+232 KB compared to latest release) ⚠️

Copy link
Contributor

github-actions bot commented Nov 8, 2024

Coverage Report

@@                       Coverage Diff                        @@
##           master   agustin-frontier-pov-reclaim      +/-   ##
================================================================
+ Coverage   74.75%                         78.98%   +4.23%     
- Files         369                            303      -66     
- Lines       94246                          88231    -6015     
================================================================
- Hits        70453                          69685     -768     
- Misses      23793                          18546    -5247     
Files Changed Coverage
/node/cli/src/cli.rs 33.33% (+0.40%) 🔼
/node/cli/src/command.rs 17.61% (+0.71%) 🔼
/node/service/src/lib.rs 62.25% (+0.23%) 🔼
/precompiles/relay-data-verifier/src/mock.rs 87.69% (+2.16%) 🔼
/runtime/moonbeam/src/xcm_config.rs 45.45% (+2.18%) 🔼
/runtime/moonbeam/tests/common/mod.rs 94.68% (-0.06%) 🔽
/runtime/moonriver/src/xcm_config.rs 45.45% (+2.18%) 🔼
/runtime/moonriver/tests/common/mod.rs 94.80% (-0.05%) 🔽

Coverage generated Wed Nov 27 14:51:19 UTC 2024

@noandrea noandrea added the A8-mergeoncegreen Pull request is reviewed well. label Nov 15, 2024
@crystalin crystalin removed the A8-mergeoncegreen Pull request is reviewed well. label Nov 18, 2024
@RomarQ RomarQ added B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes labels Nov 20, 2024
@crystalin
Copy link
Collaborator

I tested a live transaction (with lazy fork) from BeamSwap using this PR.
The cost goes from 0.48 GLMR to 0.041 GLMR !!!
(current runtime LEFT), (this PR RIGHT)
Capture d'écran 2024-11-20 134217

@crystalin
Copy link
Collaborator

@Agusrodri I can see the ref_time of weight (v2) jumps from 93M to 7M. Is that normal ?

@Agusrodri
Copy link
Contributor Author

@Agusrodri I can see the ref_time of weight (v2) jumps from 93M to 7M. Is that normal ?

Not sure if this is normal to be honest. From my understanding the refund should only take place in the pov size right? Will need to investigate.

@crystalin
Copy link
Collaborator

After more consideration, it is expected to also be lower. For Ethereum transaction, the weight (ref_time) is computed from the gas cost, using a gas to weight ratio. The gas being lower (due to PoV) the ref_time is lower being lower. So all good

@RomarQ RomarQ added D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes labels Nov 25, 2024
@RomarQ RomarQ marked this pull request as ready for review November 27, 2024 12:15
@crystalin crystalin merged commit 887e261 into master Nov 27, 2024
39 checks passed
@crystalin crystalin deleted the agustin-frontier-pov-reclaim branch November 27, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes breaking Needs to be mentioned in breaking changes D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants