[NIT-2798] Test Stylus gas usage #2708
Conversation
cla-bot
bot
added
the
s
gligneul
force-pushed
the
gligneul/program-gas-tests
branch
2 times, most recently
from
238015a
to
f0a9350
Compare
These tests compare the gas usage of EVM opcodes and Stylus HostIOs. Co-authored-by: Aman Sanghi <[email protected]> Co-authored-by: Tsahi Zidenberg <[email protected]>
gligneul
force-pushed
the
gligneul/program-gas-tests
branch
from
f0a9350
to
43e852f
Compare
| t.Cleanup(cleanup) | ||
| l2info = builder.L2Info | ||
| l2client = builder.L2.Client | ||
| auth = builder.L2Info.GetDefaultTransactOpts("Owner", ctx) |
There was a problem hiding this comment.
nit: cleaner to just return builder.
I'm in favor of tests using builder.l2info, builder.L2.Client etc directly. the very common l2info = builder.L2Info was added when we created builder
There was a problem hiding this comment.
not critical, as it's very common in other tests.
| {hostio: "contract_address", opcode: vm.ADDRESS, maxDiff: 0.5}, | ||
| {hostio: "math_div", opcode: vm.DIV, params: []any{big.NewInt(1), big.NewInt(3)}}, | ||
| {hostio: "math_mod", opcode: vm.MOD, params: []any{big.NewInt(1), big.NewInt(3)}}, | ||
| {hostio: "math_pow", opcode: vm.EXP, params: []any{big.NewInt(1), new(big.Int).Lsh(big.NewInt(1), 255)}, maxDiff: 2}, // worst case |
There was a problem hiding this comment.
maxDiff of 2 is pretty much checking nothing with current formula.
I think math_ functions will eventually be tested in our wasm gas test vs. execution time and not vs. EVM.
In the meantime, it's good to have them here, but if we don't have a good bound on math_pow vm it's better to compare it to a known constant than to EVM, with a comment explaining it.
| if maxAllowedDifference == 0 { | ||
| maxAllowedDifference = 0.25 | ||
| } | ||
| percentageDifference := math.Abs(a-b) / ((a + b) / 2) |
There was a problem hiding this comment.
The problem with this formulae is it's a little deceiving. No two numbers will give a result greater than 2.
Maybe do something like [(max(a,b) / min(a,b)) - 1]. It will be close in the relevant area but much easier to comprehend.
|
|
||
| // sendAndEnsureTransaction sends a transaction, ensures it succeed, and returns the total gas cost. | ||
| func sendAndEnsureTransaction(t *testing.T, ctx context.Context, client *ethclient.Client, tx *types.Transaction) uint64 { | ||
| t.Helper() |
There was a problem hiding this comment.
nit: if you hold on to builder, you can use builder.L2.SendWaitTestTransactions(t, )
I am hoping to have more tests build using these utils, and possibly adding more.. as it is I think we have too many variats of functions sending and ensuring transactions..
There was a problem hiding this comment.
just realised that SendWaitTestTransactions doesn't return receipt. Maybe we need a single tx function that does, or SendWaiTest could return an array of receipts.. feel free to either do something with it or ignore
These tests compare the gas usage of EVM opcodes and Stylus HostIOs.
This PR depends on the following contracts PR: OffchainLabs/nitro-contracts#255