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 ?
|
|
||
| func (r *WriteReportGasMismatch) LogAttributes() []attribute.KeyValue { | ||
| return append([]attribute.KeyValue{ | ||
| attribute.String("receiver", getReceiver(r.Req.GetReceiver())), |
There was a problem hiding this comment.
these attributes will increase the cardinality of the dataset.
Are we relying on this dataset being super small ?
Worth asking infra-obs whether a small count but high cardinality dataset is ok to have or should be avoided.
If it should be avoided, we can emit a counter and warn on increasing rate, and then just log the receiver and hash to investigate.
There was a problem hiding this comment.
I think including hash would still be a high cardinality label they'll want us to avoid. Just receiver might be ok if we only expect there to be a few
| // 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
| // MinimumGasLimit is the forwarder's routing floor: route() reverts the whole tx when the | ||
| // recorded receiver budget would fall below it (MINIMUM_GAS_LIMIT). Any transmission that | ||
| // routed at all therefore records at least this much gas. | ||
| MinimumGasLimit uint64 = InternalGasRequirements + 30_000*3 + 10_000 |
There was a problem hiding this comment.
is this used somewhere ?
| // 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 ?
|
|
||
| // warnGasMismatch reports a prior tx whose onchain gas limit differs from the requested one. | ||
| func (e *WriteReport) warnGasMismatch(ctx context.Context, telemetryContext monitoring.TelemetryContext, request *evm.WriteReportRequest, txHash evmtypes.Hash, requestedGasLimit, actualTxGasLimit uint64) { | ||
| e.lggr.Warnw("Gas mismatch: prior transmission tx gas limit does not match the requested gas limit", |
There was a problem hiding this comment.
Could we potentially report this in valid cases? If we ever enable gas limit estimation for a particular chain, the transmission gas limit may be different than what was requested. Off the top of my head, I don't think we have it enabled on our currently integrated chains but I know some exist like Hedera if we ever have to add it.
4 participants
No description provided.