feat: max fee per gas config in proposer and batch-sequencer
#876
Conversation
daniilrrr
requested review from
calebguy,
ericvelazquez,
ibremseth,
jorgemmsilva,
sammdec and
tsite
| msg, wrappedErr := logger.WrapErrorWithMsg("Failed to create settlement transactor", err) | ||
| log.Fatal().Stack().Err(wrappedErr).Msg(msg) | ||
| } | ||
| settlementAuth.GasFeeCap = big.NewInt(int64(cfg.MaxFeePerGas)) |
There was a problem hiding this comment.
think we can use new(big.Int).SetUint64(value) to avoid precision loss from uint -> int (same in other places in the proposer actually)
There was a problem hiding this comment.
good catch, thank you
There was a problem hiding this comment.
updated it to be int64 in config to not change types
|
|
||
| /// Max fee per gas unit, in wei units, for a single batch transaction submission. | ||
| /// Default is 0.0001 ETH units (SYND) / 100,000 gwei / 100000000000000 wei | ||
| #[arg(long, env = "MAX_FEE_PER_GAS", default_value = "100000000000000")] |
There was a problem hiding this comment.
Should this be optional (Option) so it defaults to None?
There was a problem hiding this comment.
chatted with @squibwarb and it's operationally easier to set a high default in the code and override it occasionally, rather than add new config values in Helm for every appchain. So I will keep it as is
| If you encounter import errors related to missing packages like `github.com/offchainlabs/nitro/solgen/go/*`, you need to regenerate the Go bindings from the Nitro contracts: | ||
|
|
||
| 1. Build legacy contracts: | ||
| ```bash | ||
| cd ../synd-enclave/nitro/contracts-legacy | ||
| yarn build | ||
| ``` | ||
|
|
||
| 2. Generate Go bindings: | ||
| ```bash | ||
| cd .. | ||
| go run solgen/gen.go | ||
| ``` | ||
|
|
||
| 3. Update Go modules: | ||
| ```bash | ||
| cd ../../synd-proposer | ||
| go mod tidy | ||
| ``` | ||
|
|
||
| **One-liner from synd-proposer directory:** | ||
| ```bash | ||
| cd ../synd-enclave/nitro/contracts-legacy && yarn build && cd .. && mkdir -p solgen/go/ && go run solgen/gen.go && cd ../../synd-proposer && go mod tidy | ||
| ``` |
There was a problem hiding this comment.
I think cd'ing into nitro dir and doing make build-node-deps does all this stuff
There was a problem hiding this comment.
I attempted exactly that but it was insufficient, hence all this stuff
There was a problem hiding this comment.
that's odd, did you get an error or something like that?
There was a problem hiding this comment.
yes. I got missing import errors relating to the solgen contracts so I had to regenerate them in the way above.
I'm happy to remove these lines, just wanted to be helpful in case others hit this issue later. The make build-node-deps command is still present as the happy path at the top of the README
There was a problem hiding this comment.
Pull Request Overview
This PR adds configurable maximum fee per gas limits for transaction submission in the proposer and batch-sequencer components to prevent transactions from being sent during extreme gas price spikes.
Key Changes:
- Added
max_fee_per_gasconfiguration parameter to both components with conservative defaults batch-sequencer: 0.0001 ETH (100,000 gwei) default limitproposer: 0.001 ETH (1,000,000 gwei) default limit
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| synd-withdrawals/synd-proposer/pkg/proposer.go | Sets gas fee cap on settlement transaction auth |
| synd-withdrawals/synd-proposer/pkg/config/config.go | Adds MaxFeePerGas config field and default value |
| synd-withdrawals/synd-proposer/README.md | Adds troubleshooting section (unrelated to PR purpose) |
| synd-batch-sequencer/src/config.rs | Adds max_fee_per_gas config field with default |
| synd-batch-sequencer/src/batcher.rs | Applies max fee per gas to transaction requests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
daniilrrr
force-pushed
the
daniil/ENG-2140-max-gas-fee
branch
from
778b325 to
19d1f65
Compare
Ticket
What does this PR do?
max_fee_per_gasconfig for these 2 components that send transactions. Default values are high so that most traffic still succeeds, aside from txns sent during extreme gas spikes.batch-sequencer: default is 0.0001 ETH units (SYND) / 100,000 gweiproposer: default is 0.001 ETH or 1,000,000 gweimax_fee_per_gasas configBreaking changes?
synd-proposer:max-fee-per-gassynd-batch-sequencer:MAX_FEE_PER_GASMetrics changes?
How can this PR be tested?