From 20dbba8e76604a2488b0717d53d706ee11b11a9c Mon Sep 17 00:00:00 2001 From: amit-momin <108959691+amit-momin@users.noreply.github.com> Date: Thu, 1 Aug 2024 08:45:14 -0500 Subject: [PATCH] Add nonce validation after broadcast for Hedera (#13957) * Added post-broadcast nonce validation for Hedera * Added changeset * Updated chaintype docs for Hedera * Fixed lint errors * Added condition to handle on-chain seq less than tx seq and updated comment --- .changeset/friendly-impalas-sniff.md | 5 + common/txmgr/broadcaster.go | 92 +++++++++++++---- common/txmgr/confirmer.go | 12 ++- common/txmgr/types/tx_store.go | 1 + core/chains/evm/config/chaintype/chaintype.go | 10 +- core/chains/evm/txmgr/broadcaster_test.go | 98 ++++++++++++++++++- core/chains/evm/txmgr/builder.go | 6 +- core/chains/evm/types/types.go | 16 +++ core/config/docs/chains-evm.toml | 2 +- core/services/chainlink/config_test.go | 4 +- core/services/ocr/contract_tracker.go | 2 +- docs/CONFIG.md | 2 +- 12 files changed, 213 insertions(+), 37 deletions(-) create mode 100644 .changeset/friendly-impalas-sniff.md diff --git a/.changeset/friendly-impalas-sniff.md b/.changeset/friendly-impalas-sniff.md new file mode 100644 index 00000000000..8a041a338bc --- /dev/null +++ b/.changeset/friendly-impalas-sniff.md @@ -0,0 +1,5 @@ +--- +"chainlink": minor +--- + +Added nonce validation immediately after broadcast for Hedera #internal diff --git a/common/txmgr/broadcaster.go b/common/txmgr/broadcaster.go index 2a9c1231d7b..b2fb1dabff7 100644 --- a/common/txmgr/broadcaster.go +++ b/common/txmgr/broadcaster.go @@ -34,6 +34,13 @@ const ( // TransmitCheckTimeout controls the maximum amount of time that will be // spent on the transmit check. TransmitCheckTimeout = 2 * time.Second + + // maxBroadcastRetries is the number of times a transaction broadcast is retried when the sequence fails to increment on Hedera + maxHederaBroadcastRetries = 3 + + // hederaChainType is the string representation of the Hedera chain type + // Temporary solution until the Broadcaster is moved to the EVM code base + hederaChainType = "hedera" ) var ( @@ -114,6 +121,7 @@ type Broadcaster[ sequenceTracker txmgrtypes.SequenceTracker[ADDR, SEQ] resumeCallback ResumeCallback chainID CHAIN_ID + chainType string config txmgrtypes.BroadcasterChainConfig feeConfig txmgrtypes.BroadcasterFeeConfig txConfig txmgrtypes.BroadcasterTransactionsConfig @@ -163,6 +171,7 @@ func NewBroadcaster[ lggr logger.Logger, checkerFactory TransmitCheckerFactory[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], autoSyncSequence bool, + chainType string, ) *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE] { lggr = logger.Named(lggr, "Broadcaster") b := &Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]{ @@ -171,6 +180,7 @@ func NewBroadcaster[ client: client, TxAttemptBuilder: txAttemptBuilder, chainID: client.ConfiguredChainID(), + chainType: chainType, config: config, feeConfig: feeConfig, txConfig: txConfig, @@ -411,7 +421,7 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) hand return fmt.Errorf("handleAnyInProgressTx failed: %w", err), true } if etx != nil { - if err, retryable := eb.handleInProgressTx(ctx, *etx, etx.TxAttempts[0], etx.CreatedAt); err != nil { + if err, retryable := eb.handleInProgressTx(ctx, *etx, etx.TxAttempts[0], etx.CreatedAt, 0); err != nil { return fmt.Errorf("handleAnyInProgressTx failed: %w", err), retryable } } @@ -464,12 +474,12 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) hand return fmt.Errorf("processUnstartedTxs failed on UpdateTxUnstartedToInProgress: %w", err), true } - return eb.handleInProgressTx(ctx, *etx, attempt, time.Now()) + return eb.handleInProgressTx(ctx, *etx, attempt, time.Now(), 0) } // There can be at most one in_progress transaction per address. // Here we complete the job that we didn't finish last time. -func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) handleInProgressTx(ctx context.Context, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], initialBroadcastAt time.Time) (error, bool) { +func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) handleInProgressTx(ctx context.Context, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], initialBroadcastAt time.Time, retryCount int) (error, bool) { if etx.State != TxInProgress { return fmt.Errorf("invariant violation: expected transaction %v to be in_progress, it was %s", etx.ID, etx.State), false } @@ -478,6 +488,11 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) hand lgr.Infow("Sending transaction", "txAttemptID", attempt.ID, "txHash", attempt.Hash, "meta", etx.Meta, "feeLimit", attempt.ChainSpecificFeeLimit, "callerProvidedFeeLimit", etx.FeeLimit, "attempt", attempt, "etx", etx) errType, err := eb.client.SendTransactionReturnCode(ctx, etx, attempt, lgr) + // The validation below is only applicable to Hedera because it has instant finality and a unique sequence behavior + if eb.chainType == hederaChainType { + errType, err = eb.validateOnChainSequence(ctx, lgr, errType, err, etx, retryCount) + } + if errType != client.Fatal { etx.InitialBroadcastAt = &initialBroadcastAt etx.BroadcastAt = &initialBroadcastAt @@ -538,7 +553,7 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) hand eb.sequenceTracker.GenerateNextSequence(etx.FromAddress, *etx.Sequence) return err, true case client.Underpriced: - return eb.tryAgainBumpingGas(ctx, lgr, err, etx, attempt, initialBroadcastAt) + return eb.tryAgainBumpingGas(ctx, lgr, err, etx, attempt, initialBroadcastAt, retryCount+1) case client.InsufficientFunds: // NOTE: This bails out of the entire cycle and essentially "blocks" on // any transaction that gets insufficient_funds. This is OK if a @@ -600,6 +615,44 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) hand } } +func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) validateOnChainSequence(ctx context.Context, lgr logger.SugaredLogger, errType client.SendTxReturnCode, err error, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], retryCount int) (client.SendTxReturnCode, error) { + // Only check if sequence was incremented if broadcast was successful, otherwise return the existing err type + if errType != client.Successful { + return errType, err + } + // Transaction sequence cannot be nil here since a sequence is required to broadcast + txSeq := *etx.Sequence + // Retrieve the latest mined sequence from on-chain + nextSeqOnChain, err := eb.client.SequenceAt(ctx, etx.FromAddress, nil) + if err != nil { + return errType, err + } + + // Check that the transaction count has incremented on-chain to include the broadcasted transaction + // Insufficient transaction fee is a common scenario in which the sequence is not incremented by the chain even though we got a successful response + // If the sequence failed to increment and hasn't reached the max retries, return the Underpriced error to try again with a bumped attempt + if nextSeqOnChain.Int64() == txSeq.Int64() && retryCount < maxHederaBroadcastRetries { + return client.Underpriced, nil + } + + // If the transaction reaches the retry limit and fails to get included, mark it as fatally errored + // Some unknown error other than insufficient tx fee could be the cause + if nextSeqOnChain.Int64() == txSeq.Int64() && retryCount >= maxHederaBroadcastRetries { + err := fmt.Errorf("failed to broadcast transaction on %s after %d retries", hederaChainType, retryCount) + lgr.Error(err.Error()) + return client.Fatal, err + } + + // Belts and braces approach to detect and handle sqeuence gaps if the broadcast is considered successful + if nextSeqOnChain.Int64() < txSeq.Int64() { + err := fmt.Errorf("next expected sequence on-chain (%s) is less than the broadcasted transaction's sequence (%s)", nextSeqOnChain.String(), txSeq.String()) + lgr.Criticalw("Sequence gap has been detected and needs to be filled", "error", err) + return client.Fatal, err + } + + return client.Successful, nil +} + // Finds next transaction in the queue, assigns a sequence, and moves it to "in_progress" state ready for broadcast. // Returns nil if no transactions are in queue func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) nextUnstartedTransactionWithSequence(fromAddress ADDR) (*txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], error) { @@ -622,23 +675,26 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) next return etx, nil } -func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) tryAgainBumpingGas(ctx context.Context, lgr logger.Logger, txError error, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], initialBroadcastAt time.Time) (err error, retryable bool) { - logger.With(lgr, - "sendError", txError, - "attemptFee", attempt.TxFee, - "maxGasPriceConfig", eb.feeConfig.MaxFeePrice(), - ).Errorf("attempt fee %v was rejected by the node for being too low. "+ - "Node returned: '%s'. "+ - "Will bump and retry. ACTION REQUIRED: This is a configuration error. "+ - "Consider increasing FeeEstimator.PriceDefault (current value: %s)", - attempt.TxFee, txError.Error(), eb.feeConfig.FeePriceDefault()) +func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) tryAgainBumpingGas(ctx context.Context, lgr logger.Logger, txError error, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], initialBroadcastAt time.Time, retry int) (err error, retryable bool) { + // This log error is not applicable to Hedera since the action required would not be needed for its gas estimator + if eb.chainType != hederaChainType { + logger.With(lgr, + "sendError", txError, + "attemptFee", attempt.TxFee, + "maxGasPriceConfig", eb.feeConfig.MaxFeePrice(), + ).Errorf("attempt fee %v was rejected by the node for being too low. "+ + "Node returned: '%s'. "+ + "Will bump and retry. ACTION REQUIRED: This is a configuration error. "+ + "Consider increasing FeeEstimator.PriceDefault (current value: %s)", + attempt.TxFee, txError.Error(), eb.feeConfig.FeePriceDefault()) + } replacementAttempt, bumpedFee, bumpedFeeLimit, retryable, err := eb.NewBumpTxAttempt(ctx, etx, attempt, nil, lgr) if err != nil { return fmt.Errorf("tryAgainBumpFee failed: %w", err), retryable } - return eb.saveTryAgainAttempt(ctx, lgr, etx, attempt, replacementAttempt, initialBroadcastAt, bumpedFee, bumpedFeeLimit) + return eb.saveTryAgainAttempt(ctx, lgr, etx, attempt, replacementAttempt, initialBroadcastAt, bumpedFee, bumpedFeeLimit, retry) } func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) tryAgainWithNewEstimation(ctx context.Context, lgr logger.Logger, txError error, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], initialBroadcastAt time.Time) (err error, retryable bool) { @@ -655,15 +711,15 @@ func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) tryA lgr.Warnw("L2 rejected transaction due to incorrect fee, re-estimated and will try again", "etxID", etx.ID, "err", err, "newGasPrice", fee, "newGasLimit", feeLimit) - return eb.saveTryAgainAttempt(ctx, lgr, etx, attempt, replacementAttempt, initialBroadcastAt, fee, feeLimit) + return eb.saveTryAgainAttempt(ctx, lgr, etx, attempt, replacementAttempt, initialBroadcastAt, fee, feeLimit, 0) } -func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) saveTryAgainAttempt(ctx context.Context, lgr logger.Logger, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], replacementAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], initialBroadcastAt time.Time, newFee FEE, newFeeLimit uint64) (err error, retyrable bool) { +func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) saveTryAgainAttempt(ctx context.Context, lgr logger.Logger, etx txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], attempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], replacementAttempt txmgrtypes.TxAttempt[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE], initialBroadcastAt time.Time, newFee FEE, newFeeLimit uint64, retry int) (err error, retyrable bool) { if err = eb.txStore.SaveReplacementInProgressAttempt(ctx, attempt, &replacementAttempt); err != nil { return fmt.Errorf("tryAgainWithNewFee failed: %w", err), true } lgr.Debugw("Bumped fee on initial send", "oldFee", attempt.TxFee.String(), "newFee", newFee.String(), "newFeeLimit", newFeeLimit) - return eb.handleInProgressTx(ctx, etx, replacementAttempt, initialBroadcastAt) + return eb.handleInProgressTx(ctx, etx, replacementAttempt, initialBroadcastAt, retry) } func (eb *Broadcaster[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) saveFatallyErroredTransaction(lgr logger.Logger, etx *txmgrtypes.Tx[CHAIN_ID, ADDR, TX_HASH, BLOCK_HASH, SEQ, FEE]) error { diff --git a/common/txmgr/confirmer.go b/common/txmgr/confirmer.go index a9e30ffff1e..1e3922fdbfb 100644 --- a/common/txmgr/confirmer.go +++ b/common/txmgr/confirmer.go @@ -664,11 +664,15 @@ func (ec *Confirmer[CHAIN_ID, HEAD, ADDR, TX_HASH, BLOCK_HASH, R, SEQ, FEE]) bat } if receipt.GetStatus() == 0 { - rpcError, errExtract := ec.client.CallContract(ctx, attempt, receipt.GetBlockNumber()) - if errExtract == nil { - l.Warnw("transaction reverted on-chain", "hash", receipt.GetTxHash(), "rpcError", rpcError.String()) + if receipt.GetRevertReason() != nil { + l.Warnw("transaction reverted on-chain", "hash", receipt.GetTxHash(), "revertReason", *receipt.GetRevertReason()) } else { - l.Warnw("transaction reverted on-chain unable to extract revert reason", "hash", receipt.GetTxHash(), "err", err) + rpcError, errExtract := ec.client.CallContract(ctx, attempt, receipt.GetBlockNumber()) + if errExtract == nil { + l.Warnw("transaction reverted on-chain", "hash", receipt.GetTxHash(), "rpcError", rpcError.String()) + } else { + l.Warnw("transaction reverted on-chain unable to extract revert reason", "hash", receipt.GetTxHash(), "err", err) + } } // This might increment more than once e.g. in case of re-orgs going back and forth we might re-fetch the same receipt promRevertedTxCount.WithLabelValues(ec.chainID.String()).Add(1) diff --git a/common/txmgr/types/tx_store.go b/common/txmgr/types/tx_store.go index 25040ea3bdb..875339cfbac 100644 --- a/common/txmgr/types/tx_store.go +++ b/common/txmgr/types/tx_store.go @@ -132,4 +132,5 @@ type ChainReceipt[TX_HASH, BLOCK_HASH types.Hashable] interface { GetFeeUsed() uint64 GetTransactionIndex() uint GetBlockHash() BLOCK_HASH + GetRevertReason() *string } diff --git a/core/chains/evm/config/chaintype/chaintype.go b/core/chains/evm/config/chaintype/chaintype.go index e8abfc5abb2..623a80f54f2 100644 --- a/core/chains/evm/config/chaintype/chaintype.go +++ b/core/chains/evm/config/chaintype/chaintype.go @@ -11,6 +11,7 @@ const ( ChainArbitrum ChainType = "arbitrum" ChainCelo ChainType = "celo" ChainGnosis ChainType = "gnosis" + ChainHedera ChainType = "hedera" ChainKroma ChainType = "kroma" ChainMetis ChainType = "metis" ChainOptimismBedrock ChainType = "optimismBedrock" @@ -19,7 +20,6 @@ const ( ChainXLayer ChainType = "xlayer" ChainZkEvm ChainType = "zkevm" ChainZkSync ChainType = "zksync" - ChainHedera ChainType = "hedera" ) // IsL2 returns true if this chain is a Layer 2 chain. Notably: @@ -36,7 +36,7 @@ func (c ChainType) IsL2() bool { func (c ChainType) IsValid() bool { switch c { - case "", ChainArbitrum, ChainCelo, ChainGnosis, ChainKroma, ChainMetis, ChainOptimismBedrock, ChainScroll, ChainWeMix, ChainXLayer, ChainZkEvm, ChainZkSync, ChainHedera: + case "", ChainArbitrum, ChainCelo, ChainGnosis, ChainHedera, ChainKroma, ChainMetis, ChainOptimismBedrock, ChainScroll, ChainWeMix, ChainXLayer, ChainZkEvm, ChainZkSync: return true } return false @@ -50,6 +50,8 @@ func ChainTypeFromSlug(slug string) ChainType { return ChainCelo case "gnosis": return ChainGnosis + case "hedera": + return ChainHedera case "kroma": return ChainKroma case "metis": @@ -66,8 +68,6 @@ func ChainTypeFromSlug(slug string) ChainType { return ChainZkEvm case "zksync": return ChainZkSync - case "hedera": - return ChainHedera default: return ChainType(slug) } @@ -123,6 +123,7 @@ var ErrInvalidChainType = fmt.Errorf("must be one of %s or omitted", strings.Joi string(ChainArbitrum), string(ChainCelo), string(ChainGnosis), + string(ChainHedera), string(ChainKroma), string(ChainMetis), string(ChainOptimismBedrock), @@ -131,5 +132,4 @@ var ErrInvalidChainType = fmt.Errorf("must be one of %s or omitted", strings.Joi string(ChainXLayer), string(ChainZkEvm), string(ChainZkSync), - string(ChainHedera), }, ", ")) diff --git a/core/chains/evm/txmgr/broadcaster_test.go b/core/chains/evm/txmgr/broadcaster_test.go index 3559c329dee..537875a6473 100644 --- a/core/chains/evm/txmgr/broadcaster_test.go +++ b/core/chains/evm/txmgr/broadcaster_test.go @@ -34,6 +34,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/chains/evm/assets" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" evmconfig "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/chaintype" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" gasmocks "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas/mocks" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/keystore" @@ -70,7 +71,7 @@ func NewTestEthBroadcaster( return gas.NewFixedPriceEstimator(config.EVM().GasEstimator(), nil, ge.BlockHistory(), lggr, nil) }, ge.EIP1559DynamicFees(), ge) txBuilder := txmgr.NewEvmTxAttemptBuilder(*ethClient.ConfiguredChainID(), ge, keyStore, estimator) - ethBroadcaster := txmgrcommon.NewBroadcaster(txStore, txmgr.NewEvmTxmClient(ethClient, nil), txmgr.NewEvmTxmConfig(config.EVM()), txmgr.NewEvmTxmFeeConfig(config.EVM().GasEstimator()), config.EVM().Transactions(), gconfig.Database().Listener(), keyStore, txBuilder, nonceTracker, lggr, checkerFactory, nonceAutoSync) + ethBroadcaster := txmgrcommon.NewBroadcaster(txStore, txmgr.NewEvmTxmClient(ethClient, nil), txmgr.NewEvmTxmConfig(config.EVM()), txmgr.NewEvmTxmFeeConfig(config.EVM().GasEstimator()), config.EVM().Transactions(), gconfig.Database().Listener(), keyStore, txBuilder, nonceTracker, lggr, checkerFactory, nonceAutoSync, "") // Mark instance as test ethBroadcaster.XXXTestDisableUnstartedTxAutoProcessing() @@ -101,6 +102,7 @@ func TestEthBroadcaster_Lifecycle(t *testing.T) { logger.Test(t), &testCheckerFactory{}, false, + "", ) // Can't close an unstarted instance @@ -159,6 +161,7 @@ func TestEthBroadcaster_LoadNextSequenceMapFailure_StartupSuccess(t *testing.T) logger.Test(t), &testCheckerFactory{}, false, + "", ) // Instance starts without error even if loading next sequence map fails @@ -638,6 +641,7 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_OptimisticLockingOnEthTx(t *testi logger.Test(t), &testCheckerFactory{}, false, + "", ) eb.XXXTestDisableUnstartedTxAutoProcessing() @@ -1157,7 +1161,7 @@ func TestEthBroadcaster_ProcessUnstartedEthTxs_Errors(t *testing.T) { }, evmcfg.EVM().GasEstimator().EIP1559DynamicFees(), evmcfg.EVM().GasEstimator()) txBuilder := txmgr.NewEvmTxAttemptBuilder(*ethClient.ConfiguredChainID(), evmcfg.EVM().GasEstimator(), ethKeyStore, estimator) localNextNonce = getLocalNextNonce(t, nonceTracker, fromAddress) - eb2 := txmgr.NewEvmBroadcaster(txStore, txmClient, txmgr.NewEvmTxmConfig(evmcfg.EVM()), txmgr.NewEvmTxmFeeConfig(evmcfg.EVM().GasEstimator()), evmcfg.EVM().Transactions(), cfg.Database().Listener(), ethKeyStore, txBuilder, lggr, &testCheckerFactory{}, false) + eb2 := txmgr.NewEvmBroadcaster(txStore, txmClient, txmgr.NewEvmTxmConfig(evmcfg.EVM()), txmgr.NewEvmTxmFeeConfig(evmcfg.EVM().GasEstimator()), evmcfg.EVM().Transactions(), cfg.Database().Listener(), ethKeyStore, txBuilder, lggr, &testCheckerFactory{}, false, "") retryable, err := eb2.ProcessUnstartedTxs(ctx, fromAddress) assert.NoError(t, err) assert.False(t, retryable) @@ -1753,7 +1757,7 @@ func TestEthBroadcaster_SyncNonce(t *testing.T) { kst.On("EnabledAddressesForChain", mock.Anything, testutils.FixtureChainID).Return(addresses, nil).Once() ethClient.On("PendingNonceAt", mock.Anything, fromAddress).Return(uint64(0), nil).Once() txmClient := txmgr.NewEvmTxmClient(ethClient, nil) - eb := txmgr.NewEvmBroadcaster(txStore, txmClient, evmTxmCfg, txmgr.NewEvmTxmFeeConfig(ge), evmcfg.EVM().Transactions(), cfg.Database().Listener(), kst, txBuilder, lggr, checkerFactory, false) + eb := txmgr.NewEvmBroadcaster(txStore, txmClient, evmTxmCfg, txmgr.NewEvmTxmFeeConfig(ge), evmcfg.EVM().Transactions(), cfg.Database().Listener(), kst, txBuilder, lggr, checkerFactory, false, "") err := eb.Start(ctx) assert.NoError(t, err) @@ -1802,6 +1806,94 @@ func TestEthBroadcaster_NonceTracker_InProgressTx(t *testing.T) { }) } +func TestEthBroadcaster_HederaBroadcastValidation(t *testing.T) { + t.Parallel() + + db := pgtest.NewSqlxDB(t) + cfg := configtest.NewTestGeneralConfig(t) + txStore := cltest.NewTestTxStore(t, db) + ethKeyStore := cltest.NewKeyStore(t, db).Eth() + evmcfg := evmtest.NewChainScopedConfig(t, cfg) + ethClient := testutils.NewEthClientMockWithDefaultChain(t) + lggr, observed := logger.TestObserved(t, zapcore.DebugLevel) + ge := evmcfg.EVM().GasEstimator() + estimator := gas.NewEvmFeeEstimator(lggr, func(lggr logger.Logger) gas.EvmEstimator { + return gas.NewFixedPriceEstimator(evmcfg.EVM().GasEstimator(), nil, ge.BlockHistory(), lggr, nil) + }, ge.EIP1559DynamicFees(), ge) + txBuilder := txmgr.NewEvmTxAttemptBuilder(*ethClient.ConfiguredChainID(), ge, ethKeyStore, estimator) + checkerFactory := &txmgr.CheckerFactory{Client: ethClient} + ctx := tests.Context(t) + + t.Run("transaction successfully broadcasted and increments on-chain nonce", func(t *testing.T) { + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + localNonce := uint64(0) + ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { + return tx.Nonce() == localNonce + }), fromAddress).Return(commonclient.Successful, nil).Once() + ethClient.On("SequenceAt", mock.Anything, fromAddress, mock.Anything).Return(evmtypes.Nonce(1), nil).Once() + + mustInsertInProgressEthTxWithAttempt(t, txStore, evmtypes.Nonce(localNonce), fromAddress) + nonceTracker := txmgr.NewNonceTracker(lggr, txStore, txmgr.NewEvmTxmClient(ethClient, nil)) + eb := txmgrcommon.NewBroadcaster(txStore, txmgr.NewEvmTxmClient(ethClient, nil), txmgr.NewEvmTxmConfig(evmcfg.EVM()), txmgr.NewEvmTxmFeeConfig(evmcfg.EVM().GasEstimator()), evmcfg.EVM().Transactions(), cfg.Database().Listener(), ethKeyStore, txBuilder, nonceTracker, lggr, checkerFactory, false, string(chaintype.ChainHedera)) + // Mark instance as test + eb.XXXTestDisableUnstartedTxAutoProcessing() + servicetest.Run(t, eb) + + retryable, err := eb.ProcessUnstartedTxs(ctx, fromAddress) + require.NoError(t, err) + require.False(t, retryable) + }) + + t.Run("transaction successfully broadcasted, failed to increment on-chain nonce, succeeded on bumped retry attempt", func(t *testing.T) { + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + localNonce := uint64(0) + ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { + return tx.Nonce() == localNonce + }), fromAddress).Return(commonclient.Successful, nil).Twice() + ethClient.On("SequenceAt", mock.Anything, fromAddress, mock.Anything).Return(evmtypes.Nonce(0), nil).Once() + ethClient.On("SequenceAt", mock.Anything, fromAddress, mock.Anything).Return(evmtypes.Nonce(1), nil).Once() + + mustInsertInProgressEthTxWithAttempt(t, txStore, evmtypes.Nonce(localNonce), fromAddress) + nonceTracker := txmgr.NewNonceTracker(lggr, txStore, txmgr.NewEvmTxmClient(ethClient, nil)) + eb := txmgrcommon.NewBroadcaster(txStore, txmgr.NewEvmTxmClient(ethClient, nil), txmgr.NewEvmTxmConfig(evmcfg.EVM()), txmgr.NewEvmTxmFeeConfig(evmcfg.EVM().GasEstimator()), evmcfg.EVM().Transactions(), cfg.Database().Listener(), ethKeyStore, txBuilder, nonceTracker, lggr, checkerFactory, false, string(chaintype.ChainHedera)) + // Mark instance as test + eb.XXXTestDisableUnstartedTxAutoProcessing() + servicetest.Run(t, eb) + + retryable, err := eb.ProcessUnstartedTxs(ctx, fromAddress) + tests.AssertLogEventually(t, observed, "Bumped fee on initial send") + require.NoError(t, err) + require.False(t, retryable) + }) + + t.Run("transaction successfully broadcasted, failed to increment on-chain nonce on every retry", func(t *testing.T) { + _, fromAddress := cltest.MustInsertRandomKey(t, ethKeyStore) + localNonce := uint64(0) + ethClient.On("SendTransactionReturnCode", mock.Anything, mock.MatchedBy(func(tx *gethTypes.Transaction) bool { + return tx.Nonce() == localNonce + }), fromAddress).Return(commonclient.Successful, nil).Times(4) + ethClient.On("SequenceAt", mock.Anything, fromAddress, mock.Anything).Return(evmtypes.Nonce(0), nil).Times(4) + + etx := mustInsertInProgressEthTxWithAttempt(t, txStore, evmtypes.Nonce(localNonce), fromAddress) + nonceTracker := txmgr.NewNonceTracker(lggr, txStore, txmgr.NewEvmTxmClient(ethClient, nil)) + eb := txmgrcommon.NewBroadcaster(txStore, txmgr.NewEvmTxmClient(ethClient, nil), txmgr.NewEvmTxmConfig(evmcfg.EVM()), txmgr.NewEvmTxmFeeConfig(evmcfg.EVM().GasEstimator()), evmcfg.EVM().Transactions(), cfg.Database().Listener(), ethKeyStore, txBuilder, nonceTracker, lggr, checkerFactory, false, string(chaintype.ChainHedera)) + // Mark instance as test + eb.XXXTestDisableUnstartedTxAutoProcessing() + servicetest.Run(t, eb) + + retryable, err := eb.ProcessUnstartedTxs(ctx, fromAddress) + tests.AssertLogEventually(t, observed, "Bumped fee on initial send") + require.NoError(t, err) + require.False(t, retryable) + tests.AssertLogEventually(t, observed, "failed to broadcast transaction on hedera after 3 retries") + + etx, err = txStore.FindTxWithAttempts(ctx, etx.ID) + require.NoError(t, err) + require.Equal(t, txmgrcommon.TxFatalError, etx.State) + require.Error(t, etx.GetError(), "failed to broadcast transaction on hedera after 3 retries") + }) +} + type testCheckerFactory struct { err error } diff --git a/core/chains/evm/txmgr/builder.go b/core/chains/evm/txmgr/builder.go index dcf15a4fa23..8234d55b960 100644 --- a/core/chains/evm/txmgr/builder.go +++ b/core/chains/evm/txmgr/builder.go @@ -10,6 +10,7 @@ import ( txmgrtypes "github.com/smartcontractkit/chainlink/v2/common/txmgr/types" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config" + "github.com/smartcontractkit/chainlink/v2/core/chains/evm/config/chaintype" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/forwarders" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/gas" "github.com/smartcontractkit/chainlink/v2/core/chains/evm/keystore" @@ -49,7 +50,7 @@ func NewTxm( feeCfg := NewEvmTxmFeeConfig(fCfg) // wrap Evm specific config txmClient := NewEvmTxmClient(client, clientErrors) // wrap Evm specific client chainID := txmClient.ConfiguredChainID() - evmBroadcaster := NewEvmBroadcaster(txStore, txmClient, txmCfg, feeCfg, txConfig, listenerConfig, keyStore, txAttemptBuilder, lggr, checker, chainConfig.NonceAutoSync()) + evmBroadcaster := NewEvmBroadcaster(txStore, txmClient, txmCfg, feeCfg, txConfig, listenerConfig, keyStore, txAttemptBuilder, lggr, checker, chainConfig.NonceAutoSync(), chainConfig.ChainType()) evmTracker := NewEvmTracker(txStore, keyStore, chainID, lggr) stuckTxDetector := NewStuckTxDetector(lggr, client.ConfiguredChainID(), chainConfig.ChainType(), fCfg.PriceMax(), txConfig.AutoPurge(), estimator, txStore, client) evmConfirmer := NewEvmConfirmer(txStore, txmClient, txmCfg, feeCfg, txConfig, dbConfig, keyStore, txAttemptBuilder, lggr, stuckTxDetector) @@ -138,7 +139,8 @@ func NewEvmBroadcaster( logger logger.Logger, checkerFactory TransmitCheckerFactory, autoSyncNonce bool, + chainType chaintype.ChainType, ) *Broadcaster { nonceTracker := NewNonceTracker(logger, txStore, client) - return txmgr.NewBroadcaster(txStore, client, chainConfig, feeConfig, txConfig, listenerConfig, keystore, txAttemptBuilder, nonceTracker, logger, checkerFactory, autoSyncNonce) + return txmgr.NewBroadcaster(txStore, client, chainConfig, feeConfig, txConfig, listenerConfig, keystore, txAttemptBuilder, nonceTracker, logger, checkerFactory, autoSyncNonce, string(chainType)) } diff --git a/core/chains/evm/types/types.go b/core/chains/evm/types/types.go index 57a53bce67a..c834ffeb866 100644 --- a/core/chains/evm/types/types.go +++ b/core/chains/evm/types/types.go @@ -63,6 +63,7 @@ type Receipt struct { BlockHash common.Hash `json:"blockHash,omitempty"` BlockNumber *big.Int `json:"blockNumber,omitempty"` TransactionIndex uint `json:"transactionIndex"` + RevertReason []byte `json:"revertReason,omitempty"` // Only provided by Hedera } // FromGethReceipt converts a gethTypes.Receipt to a Receipt @@ -86,6 +87,7 @@ func FromGethReceipt(gr *gethTypes.Receipt) *Receipt { gr.BlockHash, gr.BlockNumber, gr.TransactionIndex, + nil, } } @@ -119,6 +121,7 @@ func (r Receipt) MarshalJSON() ([]byte, error) { BlockHash common.Hash `json:"blockHash,omitempty"` BlockNumber *hexutil.Big `json:"blockNumber,omitempty"` TransactionIndex hexutil.Uint `json:"transactionIndex"` + RevertReason hexutil.Bytes `json:"revertReason,omitempty"` // Only provided by Hedera } var enc Receipt enc.PostState = r.PostState @@ -132,6 +135,7 @@ func (r Receipt) MarshalJSON() ([]byte, error) { enc.BlockHash = r.BlockHash enc.BlockNumber = (*hexutil.Big)(r.BlockNumber) enc.TransactionIndex = hexutil.Uint(r.TransactionIndex) + enc.RevertReason = r.RevertReason return json.Marshal(&enc) } @@ -149,6 +153,7 @@ func (r *Receipt) UnmarshalJSON(input []byte) error { BlockHash *common.Hash `json:"blockHash,omitempty"` BlockNumber *hexutil.Big `json:"blockNumber,omitempty"` TransactionIndex *hexutil.Uint `json:"transactionIndex"` + RevertReason *hexutil.Bytes `json:"revertReason,omitempty"` // Only provided by Hedera } var dec Receipt if err := json.Unmarshal(input, &dec); err != nil { @@ -185,6 +190,9 @@ func (r *Receipt) UnmarshalJSON(input []byte) error { if dec.TransactionIndex != nil { r.TransactionIndex = uint(*dec.TransactionIndex) } + if dec.RevertReason != nil { + r.RevertReason = *dec.RevertReason + } return nil } @@ -225,6 +233,14 @@ func (r *Receipt) GetBlockHash() common.Hash { return r.BlockHash } +func (r *Receipt) GetRevertReason() *string { + if len(r.RevertReason) == 0 { + return nil + } + revertReason := string(r.RevertReason) + return &revertReason +} + type Confirmations int const ( diff --git a/core/config/docs/chains-evm.toml b/core/config/docs/chains-evm.toml index 460f6f6500a..444804b3826 100644 --- a/core/config/docs/chains-evm.toml +++ b/core/config/docs/chains-evm.toml @@ -14,7 +14,7 @@ BlockBackfillDepth = 10 # Default # BlockBackfillSkip enables skipping of very long backfills. BlockBackfillSkip = false # Default # ChainType is automatically detected from chain ID. Set this to force a certain chain type regardless of chain ID. -# Available types: `arbitrum`, `celo`, `gnosis`, `kroma`, `metis`, `optimismBedrock`, `scroll`, `wemix`, `xlayer`, `zksync` +# Available types: `arbitrum`, `celo`, `gnosis`, `hedera`, `kroma`, `metis`, `optimismBedrock`, `scroll`, `wemix`, `xlayer`, `zksync` ChainType = 'arbitrum' # Example # FinalityDepth is the number of blocks after which an ethereum transaction is considered "final". Note that the default is automatically set based on chain ID, so it should not be necessary to change this under normal operation. # BlocksConsideredFinal determines how deeply we look back to ensure that transactions are confirmed onto the longest chain diff --git a/core/services/chainlink/config_test.go b/core/services/chainlink/config_test.go index 256ef9e9400..9b40e4dfce4 100644 --- a/core/services/chainlink/config_test.go +++ b/core/services/chainlink/config_test.go @@ -1302,7 +1302,7 @@ func TestConfig_Validate(t *testing.T) { - 1: 10 errors: - ChainType: invalid value (Foo): must not be set with this chain id - Nodes: missing: must have at least one node - - ChainType: invalid value (Foo): must be one of arbitrum, celo, gnosis, kroma, metis, optimismBedrock, scroll, wemix, xlayer, zkevm, zksync, hedera or omitted + - ChainType: invalid value (Foo): must be one of arbitrum, celo, gnosis, hedera, kroma, metis, optimismBedrock, scroll, wemix, xlayer, zkevm, zksync or omitted - HeadTracker.HistoryDepth: invalid value (30): must be greater than or equal to FinalizedBlockOffset - GasEstimator.BumpThreshold: invalid value (0): cannot be 0 if auto-purge feature is enabled for Foo - Transactions.AutoPurge.Threshold: missing: needs to be set if auto-purge feature is enabled for Foo @@ -1315,7 +1315,7 @@ func TestConfig_Validate(t *testing.T) { - 2: 5 errors: - ChainType: invalid value (Arbitrum): only "optimismBedrock" can be used with this chain id - Nodes: missing: must have at least one node - - ChainType: invalid value (Arbitrum): must be one of arbitrum, celo, gnosis, kroma, metis, optimismBedrock, scroll, wemix, xlayer, zkevm, zksync, hedera or omitted + - ChainType: invalid value (Arbitrum): must be one of arbitrum, celo, gnosis, hedera, kroma, metis, optimismBedrock, scroll, wemix, xlayer, zkevm, zksync or omitted - FinalityDepth: invalid value (0): must be greater than or equal to 1 - MinIncomingConfirmations: invalid value (0): must be greater than or equal to 1 - 3.Nodes: 5 errors: diff --git a/core/services/ocr/contract_tracker.go b/core/services/ocr/contract_tracker.go index 9546c522a2f..6651e4b65d9 100644 --- a/core/services/ocr/contract_tracker.go +++ b/core/services/ocr/contract_tracker.go @@ -399,7 +399,7 @@ func (t *OCRContractTracker) LatestBlockHeight(ctx context.Context) (blockheight // care about the block height; we have no way of getting the L1 block // height anyway return 0, nil - case "", chaintype.ChainArbitrum, chaintype.ChainCelo, chaintype.ChainGnosis, chaintype.ChainKroma, chaintype.ChainOptimismBedrock, chaintype.ChainScroll, chaintype.ChainWeMix, chaintype.ChainXLayer, chaintype.ChainZkEvm, chaintype.ChainZkSync, chaintype.ChainHedera: + case "", chaintype.ChainArbitrum, chaintype.ChainCelo, chaintype.ChainGnosis, chaintype.ChainHedera, chaintype.ChainKroma, chaintype.ChainOptimismBedrock, chaintype.ChainScroll, chaintype.ChainWeMix, chaintype.ChainXLayer, chaintype.ChainZkEvm, chaintype.ChainZkSync: // continue } latestBlockHeight := t.getLatestBlockHeight() diff --git a/docs/CONFIG.md b/docs/CONFIG.md index 0d670b3515b..240ccf1bd42 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -7346,7 +7346,7 @@ BlockBackfillSkip enables skipping of very long backfills. ChainType = 'arbitrum' # Example ``` ChainType is automatically detected from chain ID. Set this to force a certain chain type regardless of chain ID. -Available types: `arbitrum`, `celo`, `gnosis`, `kroma`, `metis`, `optimismBedrock`, `scroll`, `wemix`, `xlayer`, `zksync` +Available types: `arbitrum`, `celo`, `gnosis`, `hedera`, `kroma`, `metis`, `optimismBedrock`, `scroll`, `wemix`, `xlayer`, `zksync` ### FinalityDepth ```toml