feat: add --gas override flag to deploy and write - #404
Conversation
On Bradbury, `genlayer write` used the exact eth_estimateGas result as the outer EVM gas limit, and the transaction reverted twice before GenVM was ever reached; identical calldata succeeded when replayed with a larger explicit gas limit. The CLI exposed no way to override gas estimation for either deploy or write. Adds `--gas <gasLimit>` to both commands, threaded through to genlayer-js's writeContract/deployContract `gas` option (see genlayer-js PR genlayerlabs#205, which this depends on — this PR's tests use a mocked genlayer-js client so they don't need that change merged first, but the flag has no runtime effect against genlayer-js versions that don't understand the `gas` field yet). Added a `parseGasLimit` helper next to the existing `parseValidUntil` in fees.ts, reusing the same `parseBigNumberishOption` validation (non-negative integer, decimal or 0x-hex) plus a positive-value check specific to gas (zero isn't a meaningful gas limit). Tests: 9 new cases across tests/actions/{write,deploy}.test.ts (valid override passed through as bigint, omitted when not provided, invalid value rejected before the SDK call, zero rejected) and tests/commands/{write,deploy}.test.ts (CLI flag parsing reaches the action). Full suite: 585 passed (52 files), no regressions. TypeScript: 0 new errors in any file I touched (`npx tsc --noEmit` shows pre-existing, unrelated errors in staking/vesting/keychain code from genlayer-cli's source being ahead of its published genlayer-js dependency — none in write.ts, deploy.ts, fees.ts, or commands/index.ts).
|
This PR targeted I retargeted it to |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
Fixes the CLI-side half of #402: no way to override gas estimation for
deploy/write.On Bradbury,
genlayer writeused the exacteth_estimateGasresult as the outer EVM gas limit foraddTransaction. The outer transaction reverted twice before GenVM was ever reached; identical calldata succeeded when replayed with a larger explicit gas limit (1,319,997 estimated & reverted twice; 2,000,000 explicit & finalized).Change
Adds
--gas <gasLimit>to bothdeployandwrite, threaded through to genlayer-js'swriteContract/deployContractgasoption (see genlayer-js#205, which adds that option and this PR depends on).Added a
parseGasLimithelper next to the existingparseValidUntilinfees.ts, reusing the sameparseBigNumberishOptionvalidation (non-negative integer, decimal or0x-hex) plus a positive-value check specific to gas.Dependency note: this PR's own tests mock the
genlayer-jsclient, so they pass regardless of merge order. But the--gasflag has no actual runtime effect until genlayer-js#205 ships, since oldergenlayer-jsversions don't read agasfield at all (it's silently ignored — thewriteParams/deployParamsobjects here areany-typed, so nothing breaks, it just won't do anything yet).Testing
tests/actions/{write,deploy}.test.ts: valid override passed through as abigint, omitted when not provided, invalid value rejected before the SDK call (via the existingfailSpinnererror path, not a thrown rejection — matches how every other validation error in this codebase surfaces), zero rejected.tests/commands/{write,deploy}.test.ts: confirms the--gasCLI flag is actually parsed by commander and reaches the action.npx tsc --noEmit: 0 new errors in any file I touched (write.ts,deploy.ts,fees.ts,commands/index.ts). The existing pre-existing errors elsewhere (staking/vesting/keychain) are from this repo's source being ahead of its currently-publishedgenlayer-jsnpm dependency — unrelated to this change.