Skip to content

Conversation

wjmelements
Copy link
Collaborator

@wjmelements wjmelements commented Aug 8, 2025

The current approach is to use forge script --broadcast.
Rationale
The profile script covers some of the basic Payment operations.
The scripts are run sequentially to ensure that time has passed.

Changes

@FilOzzy FilOzzy added this to FS Aug 8, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Aug 8, 2025
@wjmelements wjmelements marked this pull request as draft August 8, 2025 19:01
@wjmelements wjmelements mentioned this pull request Aug 8, 2025
@wjmelements
Copy link
Collaborator Author

One gas variance I have seen is +1300 in the last settleRail. I do not yet know the cause. Dumping the receipts of that run here:

@wjmelements
Copy link
Collaborator Author

Seen again, uploading receipts here

One gas variance I have seen is +1300 in the last settleRail. I do not yet know the cause. Dumping the receipts of that run here:

@wjmelements
Copy link
Collaborator Author

A less-common gasUsed variation is much larger and impacts all of the transactions after the first modifyRailPayment.

-modifyRailLockup: 20489814
-modifyRailPayment: 23862826
-settleRail: 17961981
-withdraw: 21977789
-withdrawTo: 22062638
-settleRail: 27102987
-terminateRail: 19583955
+modifyRailLockup: 22141740
+modifyRailPayment: 25693249
+settleRail: 18552881
+withdraw: 22567509
+withdrawTo: 22652358
+settleRail: 33362475
+terminateRail: 20517410

tools/profile.sh Outdated

../forge-script-gas-report/forge_script_gas_report ./broadcast/Profile.s.sol/$CHAIN_ID/createRail-latest.json | tee .gas-profile

sleep 5
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sleeping works around filecoin-project/lotus#13247 by advancing to the next block.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Another workaround might be vm.etch.

@wjmelements
Copy link
Collaborator Author

A less-common gasUsed variation is much larger and impacts all of the transactions after the first modifyRailPayment.

This happens when the blockNumber unexpectedly changes between transactions (in this case after the first modifyRailPayment). This can be detected fairly easily. I will want to verify that all of the transactions in a script happen in the same block.

@wjmelements
Copy link
Collaborator Author

One gas variance I have seen is +1300 in the last settleRail.

Looks like this happens deterministically now, so that's good.

@rjan90 rjan90 moved this to ⌨️ In Progress in Payments Aug 11, 2025
@rjan90 rjan90 moved this from 📌 Triage to ⌨️ In Progress in FS Aug 11, 2025
@wjmelements
Copy link
Collaborator Author

The itest approach looks fairly simple and will probably startup and run faster.

    blockTime := 100 * time.Millisecond
    client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
    ens.InterconnectAll().BeginMining(blockTime)

Copy link

codecov bot commented Sep 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #207   +/-   ##
=======================================
  Coverage        ?   75.83%           
=======================================
  Files           ?        5           
  Lines           ?      538           
  Branches        ?      107           
=======================================
  Hits            ?      408           
  Misses          ?      125           
  Partials        ?        5           
Flag Coverage Δ
foundry 75.83% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tools/profile.sh Outdated
[[ "$(../forge-script-gas-report/forge_script_block_numbers ./broadcast/Profile.s.sol/$CHAIN_ID/createRail-latest.json | wc -l)" -eq 1 ]] || (echo possible nondeterminism detected && exit 1)
../forge-script-gas-report/forge_script_gas_report ./broadcast/Profile.s.sol/$CHAIN_ID/createRail-latest.json | tee .gas-profile

forge script -vvv -g 44000 --broadcast --chain-id $CHAIN_ID --sender $SENDER_ADDRESS --private-key $SENDER_KEY --rpc-url $API_URL/rpc/v1 --sig "settleRail(address,address,uint256)" tools/Profile.s.sol:Profile $SENDER_ADDRESS $RAIL_ADDRESS 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

one problem with the -g 44000 approach is that the FVM penalizes you for overestimating gas. Really we should do our own eth_estimateGas without simulating EVM.

Maybe we should not use forge script. That would be too bad because it has a great framework; you can use solidity and it transforms the broadcasted calls into transactions.

@wjmelements
Copy link
Collaborator Author

A new constraint on forge script viability: because it requires --skip-simulation and therefore uses eth_estimateGas, it runs each transaction sequentially, one every two blocks. Reducing the block time will be essential.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ⌨️ In Progress
Status: ⌨️ In Progress
Development

Successfully merging this pull request may close these issues.

2 participants