Repository navigation
Conversation
| WriteReportInsufficientGasRetry struct { | ||
| basic commoncapbeholder.MetricsCapBasic | ||
| } | ||
| WriteReportGasMismatch struct { |
There was a problem hiding this comment.
are we planning to add monitoring or alerting around this ?
There was a problem hiding this comment.
Yes, In a follow up
| // The minimum amount of gas that the receiver contract must get to process the forwarder report. This is the default value used when the user doesn't specify a gas limit when invoking WriteReport. | ||
| ReceiverGasMinimum uint64 `json:"receiverGasMinimum"` | ||
| ReceiverGasMinimum uint64 `json:"receiverGasMinimum"` | ||
| // Safety margin, in gas, added on top of the forwarder contract's internal gas reservation |
There was a problem hiding this comment.
you will need a corresponding CLD PR to be able to pass this through job specs
There was a problem hiding this comment.
Will do in follow up with bumped capability in core and updated regression e2e test
| // limit minus the forwarder's gas overhead, or the configured receiver gas minimum when no | ||
| // explicit limit was provided. | ||
| func (e *WriteReport) estimateReceiverGasBudget(request *evm.WriteReportRequest) uint64 { | ||
| receiverGasBudget := e.ReceiverGasMinimum + e.forwarderGasOverhead |
There was a problem hiding this comment.
how come we add forwarderGasOverhead to receiverGasBudget here ?
i might be missing something.
from my understanding, receiverGasBudget is the gas that the receiver has available to use. Not being able to understand why we add forwarderGas into that ?
There was a problem hiding this comment.
There are two options:
- User did not provide Gas, then the value we provide is forwarderGasOverhead + receiverGasMinimum. Because we want to cover our internal forwarder logic + some gas for receiver execution(the constants are aligned with what we have on chain)
- User provided gas, further in the code you can see calculations for this case
|
| receiverGasBudget = request.GasConfig.GasLimit - contracts.ForwarderContractLogicGasCost | ||
| receiverGasBudget = request.GasConfig.GasLimit - e.forwarderGasOverhead |
There was a problem hiding this comment.
Thinking about this again, shouldn't we keep contracts.ForwarderContractLogicGasCost around for backwards compatibility? Otherwise forwarderGasOverhead won't include a ForwarderGasOverheadMargin since this change introduces that field.
The other option is separately introducing ForwarderGasOverheadMargin, setting it on whichever chains need it, then adding this new logic to replace contracts.ForwarderContractLogicGasCost
4 participants
No description provided.