diff --git a/chain_capabilities/stellar/actions/write_report.go b/chain_capabilities/stellar/actions/write_report.go index a2b4b7853..3509a8506 100644 --- a/chain_capabilities/stellar/actions/write_report.go +++ b/chain_capabilities/stellar/actions/write_report.go @@ -68,12 +68,6 @@ func (s *Stellar) WriteReport( capErr := capcommon.GetError(err, isUserError) monitoring.LogAndEmitError(ctx, s.lggr, s.beholderProcessor, s.messageBuilder.BuildWriteReportError(telemetryContext, input, "Failed to WriteReport while checking if the report exists or trying to publish on chain", capErr)) - if reply != nil || len(meteringMeta.Metering) > 0 { - return &capabilities.ResponseAndMetadata[*stellarcap.WriteReportReply]{ - Response: reply, - ResponseMetadata: meteringMeta, - }, capErr - } return nil, capErr } @@ -230,8 +224,6 @@ func (wr *writeReport) execute( if err != nil { return nil, capabilities.ResponseMetadata{}, err } - ownMeteringMetadata := wr.meteringFromSubmitResponse(submitResp) - // Poll for the canonical on-chain transmission state. The forwarder may record the // outcome after the tx confirms; retry until it is visible or the context expires. postInfo, pollErr := capcommon.WithPollingRetry(ctx, wr.lggr, func(ctx context.Context) (TransmissionInfo, error) { @@ -252,21 +244,22 @@ func (wr *writeReport) execute( "localTxStatus", submitResp.TxStatus, ) - return nil, ownMeteringMetadata, errors.New("failed to retrieve transmission outcome after report submission") + return nil, capabilities.ResponseMetadata{}, errors.New("failed to retrieve transmission outcome after report submission") } switch postInfo.State { case TransmissionStateSucceeded: if submitResp.TxStatus == stellartypes.TxSuccess && submitResp.TxHash != "" { reply, err := wr.buildSuccessReply(ctx, request, telemetryContext, submitResp.TxHash) - return reply, ownMeteringMetadata, err + if err != nil { + return nil, capabilities.ResponseMetadata{}, err + } + return reply, wr.meteringFromReply(reply), nil } txHash, err := txHashRetriever.GetSuccessfulTransmissionHash(ctx) if err != nil { - // A submit occurred and was paid for; bill the local submit hash even though - // the canonical successful tx hash could not be resolved. - return nil, ownMeteringMetadata, err + return nil, capabilities.ResponseMetadata{}, err } if submitResp.TxStatus != stellartypes.TxSuccess && submitResp.TxHash != "" && submitResp.TxHash != txHash { monitoring.LogAndEmitSuccess(ctx, "Made a new transmission attempt - transmission succeeded, but local submit did not confirm (likely duplicate)", @@ -274,15 +267,18 @@ func (wr *writeReport) execute( wr.messageBuilder.BuildWriteReportDuplicateTx(telemetryContext, request, submitResp.TxHash, txHash)) } reply, err := wr.buildSuccessReply(ctx, request, telemetryContext, txHash) - return reply, ownMeteringMetadata, err + if err != nil { + return nil, capabilities.ResponseMetadata{}, err + } + return reply, wr.meteringFromReply(reply), nil case TransmissionStateFailed, TransmissionStateInvalidReceiver: if submitResp.TxStatus == stellartypes.TxSuccess && submitResp.TxHash != "" { wr.lggr.Errorw("Made a new transmission attempt - transmission failed", "txHash", submitResp.TxHash, "transmissionState", postInfo.State) reply, err := wr.buildRevertReplyFromTx(ctx, request, telemetryContext, submitResp.TxHash, postInfo, transmissionID) if err != nil { - return nil, ownMeteringMetadata, revertReplyBuildError(postInfo, transmissionID, err) + return nil, capabilities.ResponseMetadata{}, revertReplyBuildError(postInfo, transmissionID, err) } - return reply, ownMeteringMetadata, nil + return reply, wr.meteringFromReply(reply), nil } txHash, err := txHashRetriever.GetFailedTransmissionHash(ctx) @@ -292,7 +288,7 @@ func (wr *writeReport) execute( } else { wr.lggr.Errorw("Made a new transmission attempt - transmission failed, unable to retrieve failed transmission tx hash", "error", err, "localTxHash", submitResp.TxHash) } - return nil, ownMeteringMetadata, err + return nil, capabilities.ResponseMetadata{}, err } if submitResp.TxHash != "" && submitResp.TxHash != txHash { monitoring.LogAndEmitSuccess(ctx, "Made a new transmission attempt - transmission failed, but local submit hash differs from canonical failed transmission", @@ -302,13 +298,13 @@ func (wr *writeReport) execute( wr.lggr.Errorw("Made a new transmission attempt - transmission failed", "txHash", txHash, "transmissionState", postInfo.State) reply, err := wr.buildRevertReplyFromTx(ctx, request, telemetryContext, txHash, postInfo, transmissionID) if err != nil { - return nil, ownMeteringMetadata, revertReplyBuildError(postInfo, transmissionID, err) + return nil, capabilities.ResponseMetadata{}, revertReplyBuildError(postInfo, transmissionID, err) } - return reply, ownMeteringMetadata, nil + return reply, wr.meteringFromReply(reply), nil default: wr.lggr.Errorw("Invalid transmission state after submit", "state", postInfo.State, "localTxStatus", submitResp.TxStatus) wr.emitInvalidTransmissionState(ctx, request, telemetryContext, postInfo, transmissionID, "WriteReport invalid transmission state after submit", invalidTransmissionStateError(postInfo.State).Error()) - return nil, ownMeteringMetadata, invalidTransmissionStateError(postInfo.State) + return nil, capabilities.ResponseMetadata{}, invalidTransmissionStateError(postInfo.State) } } @@ -437,9 +433,9 @@ func (wr *writeReport) pollTransmissionInfo( } } -// meteringFromReply returns billing metadata carrying the on-chain transaction fee contained -// in the reply, so that nodes returning early for a prior transmission still report the gas -// spent on chain. An absent fee (unavailable from the chain) yields empty metadata. +// meteringFromReply returns billing metadata carrying the on-chain fee of the transaction the +// reply names, so every node bills the same canonical transaction whether it submitted it, +// returned early for it, or sent a duplicate. An absent fee yields empty metadata. func (wr *writeReport) meteringFromReply(reply *stellarcap.WriteReportReply) capabilities.ResponseMetadata { if reply.TransactionFee == nil { wr.lggr.Warnw("Transaction fee unavailable in reply; skipping metering", "txHash", reply.GetTxHash()) @@ -448,21 +444,6 @@ func (wr *writeReport) meteringFromReply(reply *stellarcap.WriteReportReply) cap return metering.GetResponseMetadataWriteReport(*reply.TransactionFee, wr.chainSelector) } -func (wr *writeReport) meteringFromSubmitResponse(submitResp *stellartypes.SubmitTransactionResponse) capabilities.ResponseMetadata { - if submitResp == nil { - wr.lggr.Errorw("Submitted transaction returned nil response; using zero for metering") - return metering.GetResponseMetadataWriteReport(0, wr.chainSelector) - } - if submitResp.TransactionFee == nil { - wr.lggr.Errorw("Submitted transaction fee is unavailable; using zero for metering", - "txHash", submitResp.TxHash, - "txStatus", submitResp.TxStatus, - ) - return metering.GetResponseMetadataWriteReport(0, wr.chainSelector) - } - return metering.GetResponseMetadataWriteReport(*submitResp.TransactionFee, wr.chainSelector) -} - func (wr *writeReport) checkEstimatedSpendLimit(metadata capabilities.RequestMetadata, estimatedFeeStroops uint64) error { unit := fmt.Sprintf(metering.WriteReportSpendUnitFormat, wr.chainSelector) var limitStr string diff --git a/chain_capabilities/stellar/actions/write_report_test.go b/chain_capabilities/stellar/actions/write_report_test.go index 835f142b6..aa638ad88 100644 --- a/chain_capabilities/stellar/actions/write_report_test.go +++ b/chain_capabilities/stellar/actions/write_report_test.go @@ -636,7 +636,7 @@ func TestWriteReport_EarlyReturn(t *testing.T) { func TestWriteReport_HappyPath(t *testing.T) { t.Parallel() - t.Run("fresh submit succeeds - reply uses own tx and metering uses own submit fee", func(t *testing.T) { + t.Run("fresh submit succeeds - reply and metering use the landed tx", func(t *testing.T) { t.Parallel() h := newWriteReportHelper(t) _, reqMeta, req := newWRReportFixture(t) @@ -665,7 +665,7 @@ func TestWriteReport_HappyPath(t *testing.T) { require.NotNil(t, result.Response.TransactionFee) require.Equal(t, txFee, *result.Response.TransactionFee) requireReplyBlockTimestamp(t, result.Response, testBlockTimestamp) - validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, ownFee) + validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, txFee) h.svc.AssertNotCalled(t, "GetEvents", mock.Anything, mock.Anything) }) } @@ -710,9 +710,7 @@ func TestWriteReport_Submit(t *testing.T) { result, capErr := h.stellar.WriteReport(ctx, reqMeta, req) require.NotNil(t, capErr) require.Contains(t, capErr.Error(), "failed to retrieve transmission outcome after report submission") - require.NotNil(t, result) - require.Nil(t, result.Response) - validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, testFee) + require.Nil(t, result) h.svc.AssertNotCalled(t, "GetEvents", mock.Anything, mock.Anything) }) @@ -797,9 +795,7 @@ func TestWriteReport_Submit(t *testing.T) { result, capErr := h.stellar.WriteReport(ctx, reqMeta, req) require.NotNil(t, capErr) require.Contains(t, capErr.Error(), "failed to retrieve transmission outcome after report submission") - require.NotNil(t, result) - require.Nil(t, result.Response) - validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, 0) + require.Nil(t, result) h.svc.AssertNotCalled(t, "GetEvents", mock.Anything, mock.Anything) }) @@ -834,6 +830,8 @@ func TestWriteReport_Submit(t *testing.T) { require.NotNil(t, result.Response.TxHash) require.Equal(t, testTxHash, *result.Response.TxHash) require.NotEqual(t, "mytx", *result.Response.TxHash) + // The duplicate's own fee is not billed; metering follows the landed tx like every other node. + validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, testFee) requireDuplicateTxTelemetry(t, processor.messages, "mytx", testTxHash) }) @@ -866,6 +864,7 @@ func TestWriteReport_Submit(t *testing.T) { require.NotEqual(t, "mytx", *result.Response.TxHash) require.NotNil(t, result.Response.ErrorMessage) require.Contains(t, *result.Response.ErrorMessage, "not a Wasm contract or missing on_report") + validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, testFee) requireDuplicateTxTelemetry(t, processor.messages, "mytx", testTxHash) }) } @@ -1313,9 +1312,7 @@ func TestWriteReport_TxFatalSubmitWithoutCanonicalOutcomeReturnsError(t *testing result, capErr := h.stellar.WriteReport(ctx, reqMeta, req) require.NotNil(t, capErr) require.Contains(t, capErr.Error(), "failed to retrieve transmission outcome after report submission") - require.NotNil(t, result) - require.Nil(t, result.Response) - validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, 0) + require.Nil(t, result) h.svc.AssertNotCalled(t, "GetEvents", mock.Anything, mock.Anything) } @@ -1408,9 +1405,7 @@ func TestWriteReport_PostSubmitPollFailureDoesNotRecoverFromEvents(t *testing.T) result, capErr := h.stellar.WriteReport(ctx, reqMeta, req) require.NotNil(t, capErr) require.Contains(t, capErr.Error(), "failed to retrieve transmission outcome after report submission") - require.NotNil(t, result) - require.Nil(t, result.Response) - validateWRMetering(t, result.ResponseMetadata, testWRChainSelector, testFee) + require.Nil(t, result) h.svc.AssertNotCalled(t, "GetEvents", mock.Anything, mock.Anything) } @@ -1937,30 +1932,3 @@ func TestCheckEstimatedSpendLimit(t *testing.T) { require.Contains(t, err.Error(), "invalid spend limit") }) } - -func TestMeteringFromSubmitResponse(t *testing.T) { - t.Parallel() - - t.Run("submitted fee bills own fee", func(t *testing.T) { - t.Parallel() - wr := &writeReport{ - lggr: logger.Sugared(logger.Test(t)), - chainSelector: testWRChainSelector, - } - meta := wr.meteringFromSubmitResponse(successSubmitRespWithFee(7_777)) - validateWRMetering(t, meta, testWRChainSelector, 7_777) - }) - - t.Run("nil fee bills explicit zero", func(t *testing.T) { - t.Parallel() - wr := &writeReport{ - lggr: logger.Sugared(logger.Test(t)), - chainSelector: testWRChainSelector, - } - meta := wr.meteringFromSubmitResponse(&stellartypes.SubmitTransactionResponse{ - TxStatus: stellartypes.TxSuccess, - TxHash: testTxHash, - }) - validateWRMetering(t, meta, testWRChainSelector, 0) - }) -}