Skip to content

Conversation

wjmelements
Copy link
Collaborator

@wjmelements wjmelements commented Jul 28, 2025

Reviewer @aarshkshah1992

Gas roughly measures the burden of operations on the network, so we want to reduce it.

Although filecoin gas won't be the same as Ethereum, measuring the ethereum gas will provide a way to know to what extent a change reduces gas.

Changes

  • add .gas-snapshot
  • add workflow to check the .gas-snapshot

PayeeRailsTest:testEmptyResult() (gas: 57771)
PayeeRailsTest:testGetRailsForPayeeAndToken() (gas: 149980)
PayeeRailsTest:testEmptyResult() (gas: 57793)
PayeeRailsTest:testGetManyRails() (gas: 130853979)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

looks like creating new rails is especially expensive

Copy link
Contributor

Choose a reason for hiding this comment

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

It's just a bunch of memory write operations and an event emission. Any idea why this would be ? It's all in the createRail function if you wanna take a look. (But this is not a high priority right now, totally your call man).

@rvagg
Copy link
Contributor

rvagg commented Jul 29, 2025

Some previous gas discussions for context: #127 - leading to docs/payments-gas-benchmarks.md in #175

@wjmelements
Copy link
Collaborator Author

Some previous gas discussions for context: #127 - leading to docs/payments-gas-benchmarks.md in #175

Is there an easy way to update that?

@aarshkshah1992
Copy link
Contributor

@wjmelements Wdym ? You can just open a PR to update those numbers as the doc lives in this repo (pretty sure I'm missing something here).

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Jul 29, 2025

@wjmelements

"""
Gas roughly measures the burden of operations on the network, so we want to reduce it.
"""
For my own understanding:

Dosen't it also help increase network revenue for token holders if the gas token is the same as chain token ?
All for decreasing gas as it improves UX but I've never been able to reconcile improving UX for end-users with decreasing revenue for token holders in my mental model.

@wjmelements
Copy link
Collaborator Author

@wjmelements Wdym ? You can just open a PR to update those numbers as the doc lives in this repo (pretty sure I'm missing something here).

It would be best if we could run a script (such as forge snapshot) to update those numbers automatically. Then it would be easy to see how much a PR impacts gas.

@wjmelements
Copy link
Collaborator Author

@wjmelements

"""

Gas roughly measures the burden of operations on the network, so we want to reduce it.

"""

For my own understanding:

Dosen't it also help increase network revenue for token holders if the gas token is the same as chain token ?

All for decreasing gas as it improves UX but I've never been able to reconcile improving UX for end-users with decreasing revenue for token holders in my mental model.

If the payments are more efficient, the network can do more of them, and we can supply them more sustainably and competitively. This is the supply side.

If payments are too expensive, existing and potential users will choose cheaper alternatives. This is the demand side.

@aarshkshah1992
Copy link
Contributor

@wjmelements

_It would be best if we could run a script (such as forge snapshot) to update those numbers automatically. Then it would be easy to see how much a PR impacts gas._

Thanks for your replies man ! We don't have this setup right now. But it would be awesome to have it. Please feel free to raise a PR (can do it as part of this PR as well) and we can then merge the whole thing together 👍

@wjmelements wjmelements marked this pull request as draft July 29, 2025 20:35
@wjmelements
Copy link
Collaborator Author

Closing in favor of #207

@wjmelements wjmelements closed this Aug 8, 2025
@github-project-automation github-project-automation bot moved this to 🎉 Done in Payments Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

3 participants