From 1eb696d27fbb1423058fdfdec51234717988dae4 Mon Sep 17 00:00:00 2001 From: John Saigle Date: Fri, 17 Apr 2026 11:44:29 -0400 Subject: [PATCH 1/4] node: Add watcher interface and refactor observation checks --- node/pkg/watchers/algorand/watcher.go | 100 ++++++++---- .../watchers/algorand/watcher_methods_test.go | 110 +++++++++++++ node/pkg/watchers/algorand/watcher_test.go | 66 ++++---- node/pkg/watchers/aptos/watcher.go | 110 ++++++++----- .../watchers/aptos/watcher_methods_test.go | 104 +++++++++++++ node/pkg/watchers/aptos/watcher_test.go | 9 +- node/pkg/watchers/cosmwasm/watcher.go | 65 ++++++-- .../watchers/cosmwasm/watcher_methods_test.go | 104 +++++++++++++ node/pkg/watchers/evm/reobserve.go | 39 +++-- node/pkg/watchers/evm/watcher.go | 93 +++++++---- node/pkg/watchers/evm/watcher_methods_test.go | 100 ++++++++++++ node/pkg/watchers/evm/watcher_test.go | 26 ++-- node/pkg/watchers/ibc/watcher.go | 71 +++++++-- node/pkg/watchers/ibc/watcher_methods_test.go | 104 +++++++++++++ node/pkg/watchers/mock/watcher.go | 106 +++++++++---- .../pkg/watchers/mock/watcher_methods_test.go | 98 ++++++++++++ node/pkg/watchers/near/poll.go | 2 +- node/pkg/watchers/near/tx_processing.go | 30 ++-- node/pkg/watchers/near/watcher.go | 61 ++++++-- .../pkg/watchers/near/watcher_methods_test.go | 99 ++++++++++++ node/pkg/watchers/observation.go | 110 +++++++++++++ node/pkg/watchers/observation_test.go | 135 ++++++++++++++++ node/pkg/watchers/solana/client.go | 145 +++++++++++------- node/pkg/watchers/solana/client_test.go | 16 +- node/pkg/watchers/solana/reobserve.go | 22 +-- node/pkg/watchers/solana/shim.go | 24 +-- node/pkg/watchers/solana/tx_for_addr.go | 2 +- .../watchers/solana/watcher_methods_test.go | 100 ++++++++++++ node/pkg/watchers/sui/watcher.go | 128 ++++++++++------ node/pkg/watchers/sui/watcher_methods_test.go | 99 ++++++++++++ node/pkg/watchers/sui/watcher_test.go | 7 +- node/pkg/watchers/watcher.go | 99 ++++++++++++ 32 files changed, 2010 insertions(+), 374 deletions(-) create mode 100644 node/pkg/watchers/algorand/watcher_methods_test.go create mode 100644 node/pkg/watchers/aptos/watcher_methods_test.go create mode 100644 node/pkg/watchers/cosmwasm/watcher_methods_test.go create mode 100644 node/pkg/watchers/evm/watcher_methods_test.go create mode 100644 node/pkg/watchers/ibc/watcher_methods_test.go create mode 100644 node/pkg/watchers/mock/watcher_methods_test.go create mode 100644 node/pkg/watchers/near/watcher_methods_test.go create mode 100644 node/pkg/watchers/observation.go create mode 100644 node/pkg/watchers/observation_test.go create mode 100644 node/pkg/watchers/solana/watcher_methods_test.go create mode 100644 node/pkg/watchers/sui/watcher_methods_test.go create mode 100644 node/pkg/watchers/watcher.go diff --git a/node/pkg/watchers/algorand/watcher.go b/node/pkg/watchers/algorand/watcher.go index b870d1a8a9a..3774e128de1 100644 --- a/node/pkg/watchers/algorand/watcher.go +++ b/node/pkg/watchers/algorand/watcher.go @@ -28,11 +28,12 @@ import ( ) // Algorand allows max depth of 8 inner transactions -const MAX_DEPTH = 8 +const maxDepthInnerTx = 8 type ( // Watcher is responsible for looking over Algorand blockchain and reporting new transactions to the appid Watcher struct { + chainID vaa.ChainID indexerRPC string indexerToken string algodRPC string @@ -67,6 +68,8 @@ var ( }) ) +var _ watchers.Watcher = (*Watcher)(nil) + // NewWatcher creates a new Algorand appid watcher func NewWatcher( indexerRPC string, @@ -78,6 +81,7 @@ func NewWatcher( obsvReqC <-chan *gossipv1.ObservationRequest, ) *Watcher { return &Watcher{ + chainID: vaa.ChainIDAlgorand, indexerRPC: indexerRPC, indexerToken: indexerToken, algodRPC: algodRPC, @@ -90,14 +94,50 @@ func NewWatcher( } } +func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, e.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + return validated, nil +} + +func (e *Watcher) ChainID() vaa.ChainID { + return e.chainID +} + +func (e *Watcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return fmt.Errorf("message publication is nil") + } + + e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + algorandMessagesConfirmed.Inc() + if msg.IsReobservation { + watchers.ReobservationsByChain.WithLabelValues(e.chainID.String(), "std").Inc() + } + + return nil +} + +func (e *Watcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return e.PublishMessage(msg) +} + // gatherObservations recurses through a given transactions inner-transactions // to find any messages emitted from the core wormhole contract. // Algorand allows up to 8 levels of inner transactions. func gatherObservations(e *Watcher, t types.SignedTxnWithAD, depth int, logger *zap.Logger) (obs []algorandObservation) { // SECURITY defense-in-depth: don't recurse > max depth allowed by Algorand - if depth >= MAX_DEPTH { - logger.Error("algod client", zap.Error(fmt.Errorf("exceeded max depth of %d", MAX_DEPTH))) + if depth >= maxDepthInnerTx { + logger.Error("algod client", zap.Error(fmt.Errorf("exceeded max depth of %d", maxDepthInnerTx))) return } @@ -153,7 +193,8 @@ func gatherObservations(e *Watcher, t types.SignedTxnWithAD, depth int, logger * // lookAtTxn takes an outer transaction from the block.payset and gathers // observations from messages emitted in nested inner transactions // then passes them on the relevant channels -func lookAtTxn(e *Watcher, t types.SignedTxnInBlock, b types.Block, logger *zap.Logger, isReobservation bool) { +func lookAtTxn(e *Watcher, t types.SignedTxnInBlock, b types.Block, logger *zap.Logger, validated *watchers.ValidObservation) { + isReobservation := validated != nil observations := gatherObservations(e, t.SignedTxnWithAD, 0, logger) @@ -191,22 +232,19 @@ func lookAtTxn(e *Watcher, t types.SignedTxnInBlock, b types.Block, logger *zap. IsReobservation: isReobservation, } - algorandMessagesConfirmed.Inc() - if isReobservation { - watchers.ReobservationsByChain.WithLabelValues("algorand", "std").Inc() - } + logger.Info("message observed", observation.ZapFields()...) - logger.Info("message observed", - zap.Time("timestamp", observation.Timestamp), - zap.Uint32("nonce", observation.Nonce), - zap.Uint64("sequence", observation.Sequence), - zap.Stringer("emitter_chain", observation.EmitterChain), - zap.Stringer("emitter_address", observation.EmitterAddress), - zap.Binary("payload", observation.Payload), - zap.Uint8("consistency_level", observation.ConsistencyLevel), - ) + if validated != nil { + if err := e.PublishReobservation(*validated, observation); err != nil { + logger.Error("failed to publish reobservation", zap.Error(err)) + continue + } + continue + } - e.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + if err := e.PublishMessage(observation); err != nil { + logger.Error("failed to publish message", zap.Error(err)) + } } } @@ -266,18 +304,22 @@ func (e *Watcher) Run(ctx context.Context) error { case <-ctx.Done(): return nil case r := <-e.obsvReqC: - // node/pkg/node/reobserve.go already enforces the chain id is a valid uint16 - // and only writes to the channel for this chain id. - // If either of the below cases are true, something has gone wrong - if r.ChainId > math.MaxUint16 || vaa.ChainID(r.ChainId) != vaa.ChainIDAlgorand { - panic("invalid chain ID") + validated, err := e.Validate(r) + if err != nil { + watchers.LogInvalidObservationRequest(logger, r, err) + p2p.DefaultRegistry.AddErrorCount(vaa.ChainIDAlgorand, 1) + continue } - logger.Info("Received obsv request", - zap.String("tx_hash", hex.EncodeToString(r.TxHash)), - zap.String("base32_tx_hash", base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(r.TxHash))) + logger.Info( + "received observation request", + validated.ZapFields( + zap.String("tx_hash", hex.EncodeToString(validated.TxHash())), + zap.String("base32_tx_hash", base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(validated.TxHash())), + )..., + ) - result, err := indexerClient.SearchForTransactions().TXID(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(r.TxHash)).Do(ctx) + result, err := indexerClient.SearchForTransactions().TXID(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(validated.TxHash())).Do(ctx) if err != nil { logger.Error("SearchForTransactions", zap.Error(err)) p2p.DefaultRegistry.AddErrorCount(vaa.ChainIDAlgorand, 1) @@ -294,7 +336,7 @@ func (e *Watcher) Run(ctx context.Context) error { } for _, element := range block.Payset { - lookAtTxn(e, element, block, logger, true) + lookAtTxn(e, element, block, logger, &validated) } } @@ -320,7 +362,7 @@ func (e *Watcher) Run(ctx context.Context) error { } for _, element := range block.Payset { - lookAtTxn(e, element, block, logger, false) + lookAtTxn(e, element, block, logger, nil) } e.next_round = e.next_round + 1 diff --git a/node/pkg/watchers/algorand/watcher_methods_test.go b/node/pkg/watchers/algorand/watcher_methods_test.go new file mode 100644 index 00000000000..d0f0ca65e1e --- /dev/null +++ b/node/pkg/watchers/algorand/watcher_methods_test.go @@ -0,0 +1,110 @@ +package algorand + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newMethodTestWatcher(msgC chan<- *common.MessagePublication) *Watcher { + return &Watcher{chainID: vaa.ChainIDAlgorand, msgC: msgC} +} + +func TestWatcherChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{ + {name: "returns configured chain", want: vaa.ChainIDAlgorand}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := newMethodTestWatcher(make(chan *common.MessagePublication, 1)) + assert.Equal(t, tt.want, w.ChainID()) + }) + } +} + +func TestWatcherValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand), TxHash: make([]byte, common.TxIDLenMin)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}, wantErr: true}, + {name: "accepts empty tx hash", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand)}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := newMethodTestWatcher(make(chan *common.MessagePublication, 1)) + validated, err := w.Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDAlgorand}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + w := newMethodTestWatcher(msgC) + err := w.PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherPublishReobservation(t *testing.T) { + validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand), TxHash: make([]byte, common.TxIDLenMin)}) + require.NoError(t, err) + + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDAlgorand}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + w := newMethodTestWatcher(msgC) + err := w.PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/algorand/watcher_test.go b/node/pkg/watchers/algorand/watcher_test.go index 8c6c6920517..075c6a6d312 100644 --- a/node/pkg/watchers/algorand/watcher_test.go +++ b/node/pkg/watchers/algorand/watcher_test.go @@ -11,18 +11,19 @@ import ( "github.com/algorand/go-algorand-sdk/types" "github.com/certusone/wormhole/node/pkg/common" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/certusone/wormhole/node/pkg/watchers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/wormhole-foundation/wormhole/sdk/vaa" "go.uber.org/zap" ) -const APP_ID = 86525623 +const appID = 86525623 // helper to create a watcher for testing. func newTestWatcher(msgC chan *common.MessagePublication) *Watcher { obsvReqC := make(chan *gossipv1.ObservationRequest, 50) - return NewWatcher("", "", "", "", APP_ID, msgC, obsvReqC) + return NewWatcher("", "", "", "", appID, msgC, obsvReqC) } // helper to build a valid 8-byte big-endian encoding of a uint64. @@ -84,21 +85,21 @@ func TestGatherObservations(t *testing.T) { }{ { name: "valid 8-byte nonce and sequence", - txn: makePublishTxn(APP_ID, uint64Bytes(7), []byte("hello"), string(uint64Bytes(42))), + txn: makePublishTxn(appID, uint64Bytes(7), []byte("hello"), string(uint64Bytes(42))), expectedCount: 1, expectedNonce: 7, expectedSeq: 42, }, { name: "valid zero nonce and zero sequence", - txn: makePublishTxn(APP_ID, uint64Bytes(0), []byte(""), string(uint64Bytes(0))), + txn: makePublishTxn(appID, uint64Bytes(0), []byte(""), string(uint64Bytes(0))), expectedCount: 1, expectedNonce: 0, expectedSeq: 0, }, { name: "valid max uint32 nonce", - txn: makePublishTxn(APP_ID, uint64Bytes(0xFFFFFFFF), []byte("payload"), string(uint64Bytes(100))), + txn: makePublishTxn(appID, uint64Bytes(0xFFFFFFFF), []byte("payload"), string(uint64Bytes(100))), expectedCount: 1, expectedNonce: 0xFFFFFFFF, expectedSeq: 100, @@ -111,7 +112,7 @@ func TestGatherObservations(t *testing.T) { { name: "wrong method name", txn: func() types.SignedTxnWithAD { - txn := makePublishTxn(APP_ID, uint64Bytes(1), []byte("hello"), string(uint64Bytes(1))) + txn := makePublishTxn(appID, uint64Bytes(1), []byte("hello"), string(uint64Bytes(1))) txn.Txn.ApplicationArgs[0] = []byte("notPublishMessage") return txn }(), @@ -121,7 +122,7 @@ func TestGatherObservations(t *testing.T) { name: "too few application args (2 instead of 3)", txn: func() types.SignedTxnWithAD { txn := types.SignedTxnWithAD{} - txn.Txn.ApplicationID = types.AppIndex(APP_ID) + txn.Txn.ApplicationID = types.AppIndex(appID) txn.Txn.ApplicationArgs = [][]byte{ []byte("publishMessage"), []byte("hello"), @@ -135,7 +136,7 @@ func TestGatherObservations(t *testing.T) { name: "too many application args (4 instead of 3)", txn: func() types.SignedTxnWithAD { txn := types.SignedTxnWithAD{} - txn.Txn.ApplicationID = types.AppIndex(APP_ID) + txn.Txn.ApplicationID = types.AppIndex(appID) txn.Txn.ApplicationArgs = [][]byte{ []byte("publishMessage"), []byte("hello"), @@ -150,7 +151,7 @@ func TestGatherObservations(t *testing.T) { { name: "no logs", txn: func() types.SignedTxnWithAD { - txn := makePublishTxn(APP_ID, uint64Bytes(1), []byte("hello"), "") + txn := makePublishTxn(appID, uint64Bytes(1), []byte("hello"), "") txn.EvalDelta.Logs = nil return txn }(), @@ -204,7 +205,7 @@ func TestGatherObservations_InvalidLengths(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) w := newTestWatcher(msgC) - txn := makePublishTxn(APP_ID, tc.nonce, []byte("payload"), tc.seq) + txn := makePublishTxn(appID, tc.nonce, []byte("payload"), tc.seq) // Must not panic. obs := gatherObservations(w, txn, 0, logger) @@ -218,24 +219,24 @@ func TestGatherObservations_MaxDepth(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) w := newTestWatcher(msgC) - // Build a chain of nested inner transactions at exactly MAX_DEPTH. + // Build a chain of nested inner transactions at exactly maxDepthInnerTx. // The valid publishMessage sits at the deepest level. - validTxn := makePublishTxn(APP_ID, uint64Bytes(1), []byte("deep"), string(uint64Bytes(99))) + validTxn := makePublishTxn(appID, uint64Bytes(1), []byte("deep"), string(uint64Bytes(99))) - // Nest it MAX_DEPTH levels deep (should be rejected). + // Nest it maxDepthInnerTx levels deep (should be rejected). nested := validTxn - for i := 0; i < MAX_DEPTH; i++ { + for i := 0; i < maxDepthInnerTx; i++ { wrapper := types.SignedTxnWithAD{} wrapper.EvalDelta.InnerTxns = []types.SignedTxnWithAD{nested} nested = wrapper } obs := gatherObservations(w, nested, 0, logger) - assert.Empty(t, obs, "should not observe messages beyond MAX_DEPTH") + assert.Empty(t, obs, "should not observe messages beyond maxDepthInnerTx") - // Nest it MAX_DEPTH-1 levels deep (should succeed). + // Nest it maxDepthInnerTx-1 levels deep (should succeed). nested = validTxn - for i := 0; i < MAX_DEPTH-1; i++ { + for i := 0; i < maxDepthInnerTx-1; i++ { wrapper := types.SignedTxnWithAD{} wrapper.EvalDelta.InnerTxns = []types.SignedTxnWithAD{nested} nested = wrapper @@ -252,8 +253,8 @@ func TestGatherObservations_MultipleInnerTxns(t *testing.T) { w := newTestWatcher(msgC) // Two valid publishMessage inner transactions at the same level. - txn1 := makePublishTxn(APP_ID, uint64Bytes(10), []byte("first"), string(uint64Bytes(100))) - txn2 := makePublishTxn(APP_ID, uint64Bytes(20), []byte("second"), string(uint64Bytes(200))) + txn1 := makePublishTxn(appID, uint64Bytes(10), []byte("first"), string(uint64Bytes(100))) + txn2 := makePublishTxn(appID, uint64Bytes(20), []byte("second"), string(uint64Bytes(200))) wrapper := types.SignedTxnWithAD{} wrapper.EvalDelta.InnerTxns = []types.SignedTxnWithAD{txn1, txn2} @@ -273,9 +274,9 @@ func TestGatherObservations_MixedValidAndInvalid(t *testing.T) { w := newTestWatcher(msgC) // One valid, one with a short nonce, one valid again. - valid1 := makePublishTxn(APP_ID, uint64Bytes(1), []byte("ok1"), string(uint64Bytes(10))) - invalid := makePublishTxn(APP_ID, []byte{0xBA, 0xD0}, []byte("bad"), string(uint64Bytes(20))) - valid2 := makePublishTxn(APP_ID, uint64Bytes(3), []byte("ok2"), string(uint64Bytes(30))) + valid1 := makePublishTxn(appID, uint64Bytes(1), []byte("ok1"), string(uint64Bytes(10))) + invalid := makePublishTxn(appID, []byte{0xBA, 0xD0}, []byte("bad"), string(uint64Bytes(20))) + valid2 := makePublishTxn(appID, uint64Bytes(3), []byte("ok2"), string(uint64Bytes(30))) wrapper := types.SignedTxnWithAD{} wrapper.EvalDelta.InnerTxns = []types.SignedTxnWithAD{valid1, invalid, valid2} @@ -293,7 +294,7 @@ func TestLookAtTxn_PublishesToMsgC(t *testing.T) { logger, _ := zap.NewProduction() txn := types.SignedTxnInBlock{} - txn.SignedTxnWithAD = makePublishTxn(APP_ID, uint64Bytes(5), []byte("test payload"), string(uint64Bytes(77))) + txn.SignedTxnWithAD = makePublishTxn(appID, uint64Bytes(5), []byte("test payload"), string(uint64Bytes(77))) block := types.Block{ BlockHeader: types.BlockHeader{ @@ -302,7 +303,7 @@ func TestLookAtTxn_PublishesToMsgC(t *testing.T) { }, } - lookAtTxn(w, txn, block, logger, false) + lookAtTxn(w, txn, block, logger, nil) select { case msg := <-msgC: @@ -324,13 +325,16 @@ func TestLookAtTxn_Reobservation(t *testing.T) { logger, _ := zap.NewProduction() txn := types.SignedTxnInBlock{} - txn.SignedTxnWithAD = makePublishTxn(APP_ID, uint64Bytes(1), []byte("reobs"), string(uint64Bytes(1))) + txn.SignedTxnWithAD = makePublishTxn(appID, uint64Bytes(1), []byte("reobs"), string(uint64Bytes(1))) block := types.Block{ BlockHeader: types.BlockHeader{TimeStamp: 1700000000}, } - lookAtTxn(w, txn, block, logger, true) + validated, err := watchers.ValidateObservationRequest(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand), TxHash: make([]byte, common.TxIDLenMin)}, vaa.ChainIDAlgorand) + require.NoError(t, err) + + lookAtTxn(w, txn, block, logger, &validated) select { case msg := <-msgC: @@ -351,7 +355,7 @@ func TestLookAtTxn_NoMatchProducesNoMessage(t *testing.T) { block := types.Block{} - lookAtTxn(w, txn, block, logger, false) + lookAtTxn(w, txn, block, logger, nil) select { case <-msgC: @@ -368,13 +372,13 @@ func TestLookAtTxn_InvalidNonceDoesNotBlock(t *testing.T) { // Transaction with a short nonce -- must not panic or block. txn := types.SignedTxnInBlock{} - txn.SignedTxnWithAD = makePublishTxn(APP_ID, []byte{0xDE, 0xAD}, []byte("bad"), string(uint64Bytes(1))) + txn.SignedTxnWithAD = makePublishTxn(appID, []byte{0xDE, 0xAD}, []byte("bad"), string(uint64Bytes(1))) block := types.Block{ BlockHeader: types.BlockHeader{TimeStamp: 1700000000}, } - lookAtTxn(w, txn, block, logger, false) + lookAtTxn(w, txn, block, logger, nil) select { case <-msgC: @@ -389,7 +393,7 @@ func TestGatherObservations_EmitterAddress(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) w := newTestWatcher(msgC) - txn := makePublishTxn(APP_ID, uint64Bytes(0), []byte(""), string(uint64Bytes(0))) + txn := makePublishTxn(appID, uint64Bytes(0), []byte(""), string(uint64Bytes(0))) // Set a known sender address. var sender types.Address @@ -413,7 +417,7 @@ func TestGatherObservations_PayloadPreserved(t *testing.T) { w := newTestWatcher(msgC) payload := []byte{0x01, 0x00, 0xFF, 0xAB, 0xCD} - txn := makePublishTxn(APP_ID, uint64Bytes(0), payload, string(uint64Bytes(0))) + txn := makePublishTxn(appID, uint64Bytes(0), payload, string(uint64Bytes(0))) obs := gatherObservations(w, txn, 0, logger) diff --git a/node/pkg/watchers/aptos/watcher.go b/node/pkg/watchers/aptos/watcher.go index 529e7d0b565..dc0cddcf8f3 100644 --- a/node/pkg/watchers/aptos/watcher.go +++ b/node/pkg/watchers/aptos/watcher.go @@ -54,6 +54,8 @@ var ( }, []string{"chain_name"}) ) +var _ watchers.Watcher = (*Watcher)(nil) + // NewWatcher creates a new Aptos appid watcher func NewWatcher( chainID vaa.ChainID, @@ -76,6 +78,54 @@ func NewWatcher( } } +func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, e.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + // Aptos's TxID is a uint64. Historically, all TxIDs used a fixed 32-byte hash type. + // This parsing is leftover from that time period. It should be possible to refactor + // this code such that the TxID received from p2p is exactly 8 bytes, which would + // obviate the need for the below bounds check and parsing. + // + // SECURITY: This acts as a bounds check for the BigEndian.Uint64 call in the + // reobservation handler. + const aptosTxIDExpectedLen = 32 + if len(validated.TxHash()) < aptosTxIDExpectedLen { + return watchers.ValidObservation{}, fmt.Errorf("invalid TxID: too short") + } + + return validated, nil +} + +func (e *Watcher) ChainID() vaa.ChainID { + return e.chainID +} + +func (e *Watcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return fmt.Errorf("message publication is nil") + } + + e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + aptosMessagesConfirmed.WithLabelValues(e.networkID).Inc() + if msg.IsReobservation { + watchers.ReobservationsByChain.WithLabelValues(e.chainID.String(), "std").Inc() + } + + return nil +} + +func (e *Watcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return e.PublishMessage(msg) +} + func (e *Watcher) Run(ctx context.Context) error { p2p.DefaultRegistry.SetNetworkStats(e.chainID, &gossipv1.Heartbeat_Network{ ContractAddress: e.aptosAccount, @@ -120,30 +170,19 @@ func (e *Watcher) Run(ctx context.Context) error { case <-ctx.Done(): return ctx.Err() case r := <-e.obsvReqC: - // node/pkg/node/reobserve.go already enforces the chain id is a valid uint16 - // and only writes to the channel for this chain id. - // If either of the below cases are true, something has gone wrong - if r.ChainId > math.MaxUint16 || vaa.ChainID(r.ChainId) != e.chainID { - panic("invalid chain ID") - } - - // Aptos's TxID is a uint64. Historically, all TxIDs used a fixed 32-byte hash type. - // This parsing is leftover from that time period. It should be possible to refactor - // this code such that the TxID received from p2p is exactly 8 bytes, which would - // obviate the need for the below bounds check and parsing. - // - // SECURITY: This acts as a bounds check for the BigEndian.Unint64 call below. - const AptosTxIDExpectedLen = 32 - if len(r.TxHash) < AptosTxIDExpectedLen { - logger.Error("invalid TxID: too short") + validated, err := e.Validate(r) + if err != nil { + watchers.LogInvalidObservationRequest(logger, r, err) p2p.DefaultRegistry.AddErrorCount(e.chainID, 1) continue } + txHash := validated.TxHash() + // uint64 will read the *first* 8 bytes, but the sequence is stored in the *last* 8. - nativeSeq := binary.BigEndian.Uint64(r.TxHash[24:]) + nativeSeq := binary.BigEndian.Uint64(txHash[24:]) - logger.Info("Received obsv request", zap.Uint64("tx_hash", nativeSeq)) + logger.Info("received observation request", validated.ZapFields(zap.Uint64("tx_hash", nativeSeq))...) s := fmt.Sprintf(`%s?start=%d&limit=1`, eventsEndpoint, nativeSeq) @@ -179,7 +218,7 @@ func (e *Watcher) Run(ctx context.Context) error { if !data.Exists() { break } - e.observeData(logger, data, nativeSeq, true) + e.observeData(logger, data, nativeSeq, &validated) } case <-timer.C: @@ -241,7 +280,7 @@ func (e *Watcher) Run(ctx context.Context) error { if !data.Exists() { continue } - e.observeData(logger, data, eventSeq, false) + e.observeData(logger, data, eventSeq, nil) } health, err := e.retrievePayload(aptosHealth) @@ -299,7 +338,8 @@ func (e *Watcher) retrievePayload(s string) ([]byte, error) { return body, err } -func (e *Watcher) observeData(logger *zap.Logger, data gjson.Result, nativeSeq uint64, isReobservation bool) { +func (w *Watcher) observeData(logger *zap.Logger, data gjson.Result, nativeSeq uint64, validated *watchers.ValidObservation) { + isReobservation := validated != nil em := data.Get("sender") if !em.Exists() { logger.Error("sender field missing") @@ -375,30 +415,26 @@ func (e *Watcher) observeData(logger *zap.Logger, data gjson.Result, nativeSeq u Timestamp: time.Unix(int64(ts.Uint()), 0), // #nosec G115 -- This conversion is safe indefinitely Nonce: uint32(nonce.Uint()), // #nosec G115 -- This is validated above Sequence: sequence.Uint(), - EmitterChain: e.chainID, + EmitterChain: w.chainID, EmitterAddress: a, Payload: pl, ConsistencyLevel: uint8(consistencyLevel.Uint()), // #nosec G115 -- This is validated above IsReobservation: isReobservation, } - aptosMessagesConfirmed.WithLabelValues(e.networkID).Inc() - if isReobservation { - watchers.ReobservationsByChain.WithLabelValues(e.chainID.String(), "std").Inc() - } + logger.Info("message observed", observation.ZapFields(zap.String("txHash", observation.TxIDString()), zap.Uint8("consistencyLevel", observation.ConsistencyLevel))...) - logger.Info("message observed", - zap.String("txHash", observation.TxIDString()), - zap.Time("timestamp", observation.Timestamp), - zap.Uint32("nonce", observation.Nonce), - zap.Uint64("sequence", observation.Sequence), - zap.Stringer("emitter_chain", observation.EmitterChain), - zap.Stringer("emitter_address", observation.EmitterAddress), - zap.Binary("payload", observation.Payload), - zap.Uint8("consistencyLevel", observation.ConsistencyLevel), - ) + if validated != nil { + if err := w.PublishReobservation(*validated, observation); err != nil { + logger.Error("failed to publish reobservation", zap.Error(err)) + return + } + return + } - e.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + if err := w.PublishMessage(observation); err != nil { + logger.Error("failed to publish message", zap.Error(err)) + } } // logVersion retrieves the Aptos node version and logs it diff --git a/node/pkg/watchers/aptos/watcher_methods_test.go b/node/pkg/watchers/aptos/watcher_methods_test.go new file mode 100644 index 00000000000..c70f8b40672 --- /dev/null +++ b/node/pkg/watchers/aptos/watcher_methods_test.go @@ -0,0 +1,104 @@ +package aptos + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newTestWatcher(msgC chan<- *common.MessagePublication) *Watcher { + return &Watcher{chainID: vaa.ChainIDAptos, networkID: "aptos", msgC: msgC} +} + +func TestWatcherChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDAptos}} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newTestWatcher(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestWatcherValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, 32)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, 32)}, wantErr: true}, + {name: "rejects too short aptos tx id", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, 31)}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDAptos}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherPublishReobservation(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, 32)}) + require.NoError(t, err) + + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDAptos}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/aptos/watcher_test.go b/node/pkg/watchers/aptos/watcher_test.go index 074ae9000a1..d2245e14b60 100644 --- a/node/pkg/watchers/aptos/watcher_test.go +++ b/node/pkg/watchers/aptos/watcher_test.go @@ -6,6 +6,8 @@ import ( "testing" "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/certusone/wormhole/node/pkg/watchers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tidwall/gjson" @@ -162,7 +164,7 @@ func TestObserveData(t *testing.T) { // Must not panic for any input. require.NotPanics(t, func() { - w.observeData(logger, gjson.Parse(tc.json), tc.nativeSeq, false) + w.observeData(logger, gjson.Parse(tc.json), tc.nativeSeq, nil) }) if tc.expectError != "" { @@ -199,7 +201,10 @@ func TestObserveDataFields(t *testing.T) { "consistency_level": "15" }` - w.observeData(logger, gjson.Parse(json), 123, true) + validated, err := watchers.ValidateObservationRequest(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, common.TxIDLenMin)}, vaa.ChainIDAptos) + require.NoError(t, err) + + w.observeData(logger, gjson.Parse(json), 123, &validated) require.Len(t, msgC, 1) msg := <-msgC diff --git a/node/pkg/watchers/cosmwasm/watcher.go b/node/pkg/watchers/cosmwasm/watcher.go index 38f380a5166..930f0b0ef2b 100644 --- a/node/pkg/watchers/cosmwasm/watcher.go +++ b/node/pkg/watchers/cosmwasm/watcher.go @@ -5,7 +5,6 @@ import ( "encoding/base64" "encoding/hex" "fmt" - "math" "net/http" "strconv" "time" @@ -91,6 +90,8 @@ var ( }, []string{"terra_network", "operation"}) ) +var _ watchers.Watcher = (*Watcher)(nil) + type clientRequest struct { JSONRPC string `json:"jsonrpc"` // A String containing the name of the method to be invoked. @@ -144,6 +145,42 @@ func NewWatcher( } } +func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, e.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + return validated, nil +} + +func (e *Watcher) ChainID() vaa.ChainID { + return e.chainID +} + +func (e *Watcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return fmt.Errorf("message publication is nil") + } + + e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + messagesConfirmed.WithLabelValues(e.networkName).Inc() + if msg.IsReobservation { + watchers.ReobservationsByChain.WithLabelValues(e.networkName, "std").Inc() + } + + return nil +} + +func (e *Watcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return e.PublishMessage(msg) +} + func (e *Watcher) Run(ctx context.Context) error { p2p.DefaultRegistry.SetNetworkStats(e.chainID, &gossipv1.Heartbeat_Network{ ContractAddress: e.contract, @@ -251,19 +288,18 @@ func (e *Watcher) Run(ctx context.Context) error { case <-ctx.Done(): return nil case r := <-e.obsvReqC: - // node/pkg/node/reobserve.go already enforces the chain id is a valid uint16 - // and only writes to the channel for this chain id. - // If either of the below cases are true, something has gone wrong - if r.ChainId > math.MaxUint16 || vaa.ChainID(r.ChainId) != e.chainID { - panic("invalid chain ID") + validated, err := e.Validate(r) + if err != nil { + watchers.LogInvalidObservationRequest(logger, r, err) + continue } // SECURITY: Directly using data for URL path is scary. // Potential for directory traversal attacks to return the incorrect data // This is hex encoded so it's acceptable but be careful changing this logic. - tx := hex.EncodeToString(r.TxHash) + tx := hex.EncodeToString(validated.TxHash()) - logger.Info("received observation request", zap.String("network", e.networkName), zap.String("tx_hash", tx)) + logger.Info("received observation request", validated.ZapFields(zap.String("network", e.networkName), zap.String("tx_hash", tx))...) client := &http.Client{ Timeout: time.Second * 5, @@ -315,10 +351,10 @@ func (e *Watcher) Run(ctx context.Context) error { msgs := EventsToMessagePublications(e.contract, txHash, events.Array(), logger, e.chainID, contractAddressLogKey, e.b64Encoded) for _, msg := range msgs { - msg.IsReobservation = true - e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - messagesConfirmed.WithLabelValues(e.networkName).Inc() - watchers.ReobservationsByChain.WithLabelValues(e.networkName, "std").Inc() + if err := e.PublishReobservation(validated, msg); err != nil { + logger.Error("failed to publish reobservation", zap.Error(err)) + continue + } } } } @@ -357,8 +393,9 @@ func (e *Watcher) Run(ctx context.Context) error { msgs := EventsToMessagePublications(e.contract, txHash, events.Array(), logger, e.chainID, e.contractAddressLogKey, e.b64Encoded) for _, msg := range msgs { - e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - messagesConfirmed.WithLabelValues(e.networkName).Inc() + if err := e.PublishMessage(msg); err != nil { + logger.Error("failed to publish message", zap.Error(err)) + } } // We do not send guardian changes to the processor - ETH guardians are the source of truth. diff --git a/node/pkg/watchers/cosmwasm/watcher_methods_test.go b/node/pkg/watchers/cosmwasm/watcher_methods_test.go new file mode 100644 index 00000000000..b30503529b2 --- /dev/null +++ b/node/pkg/watchers/cosmwasm/watcher_methods_test.go @@ -0,0 +1,104 @@ +package cosmwasm + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newTestWatcher(msgC chan<- *common.MessagePublication) *Watcher { + return &Watcher{chainID: vaa.ChainIDTerra, networkName: "terra", msgC: msgC} +} + +func TestWatcherChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDTerra}} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newTestWatcher(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestWatcherValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDTerra), TxHash: make([]byte, common.TxIDLenMin)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}, wantErr: true}, + {name: "accepts empty tx hash", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDTerra)}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDTerra}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherPublishReobservation(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDTerra), TxHash: make([]byte, common.TxIDLenMin)}) + require.NoError(t, err) + + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDTerra}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/evm/reobserve.go b/node/pkg/watchers/evm/reobserve.go index bbfec5602e7..1445d573eb6 100644 --- a/node/pkg/watchers/evm/reobserve.go +++ b/node/pkg/watchers/evm/reobserve.go @@ -5,6 +5,8 @@ import ( "fmt" "time" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/certusone/wormhole/node/pkg/watchers" "github.com/certusone/wormhole/node/pkg/watchers/evm/connectors" eth_common "github.com/ethereum/go-ethereum/common" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -12,15 +14,9 @@ import ( ) // handleReobservationRequest performs a reobservation request and publishes any observed transactions. -func (w *Watcher) handleReobservationRequest(ctx context.Context, chainId vaa.ChainID, txID []byte, ethConn connectors.Connector, finalizedBlockNum, safeBlockNum uint64) (numObservations uint32, err error) { - // This can't happen unless there is a programming error - the caller - // is expected to send us only requests for our chainID. - if chainId != w.chainID { - return 0, fmt.Errorf("unexpected chain id: %v", chainId) - } - - tx := eth_common.BytesToHash(txID) - w.logger.Info("received observation request", zap.String("tx_hash", tx.Hex())) +func (w *Watcher) handleReobservationRequest(ctx context.Context, observation watchers.ValidObservation, ethConn connectors.Connector, finalizedBlockNum, safeBlockNum uint64) (numObservations uint32, err error) { + tx := eth_common.BytesToHash(observation.TxHash()) + w.logger.Info("received observation request", observation.ZapFields(zap.String("tx_hash", tx.Hex()))...) // SECURITY: We loaded the block number before requesting the transaction to avoid a // race condition where requesting the tx succeeds and is then dropped due to a fork, @@ -39,7 +35,6 @@ func (w *Watcher) handleReobservationRequest(ctx context.Context, chainId vaa.Ch } for _, msg := range msgs { - msg.IsReobservation = true if msg.ConsistencyLevel == vaa.ConsistencyLevelPublishImmediately { w.logger.Info("re-observed message publication transaction, publishing it immediately", zap.String("msgId", msg.MessageIDString()), @@ -48,7 +43,11 @@ func (w *Watcher) handleReobservationRequest(ctx context.Context, chainId vaa.Ch zap.Uint64("observed_block", blockNumber), ) - pubErr := w.verifyAndPublish(msg, ctx, eth_common.BytesToHash(msg.TxID), receipt) + verifiedMsg, verifyErr := w.verifyMessage(msg, ctx, eth_common.BytesToHash(msg.TxID), receipt) + pubErr := verifyErr + if pubErr == nil { + pubErr = w.PublishReobservation(observation, verifiedMsg) + } if pubErr != nil { w.logger.Error("Error when publishing message", zap.Error(pubErr)) @@ -76,7 +75,11 @@ func (w *Watcher) handleReobservationRequest(ctx context.Context, chainId vaa.Ch zap.Uint64("observed_block", blockNumber), ) - pubErr := w.verifyAndPublish(msg, ctx, eth_common.BytesToHash(msg.TxID), receipt) + verifiedMsg, verifyErr := w.verifyMessage(msg, ctx, eth_common.BytesToHash(msg.TxID), receipt) + pubErr := verifyErr + if pubErr == nil { + pubErr = w.PublishReobservation(observation, verifiedMsg) + } if pubErr != nil { w.logger.Error("Error when publishing message", zap.Error(pubErr)) @@ -122,7 +125,11 @@ func (w *Watcher) handleReobservationRequest(ctx context.Context, chainId vaa.Ch zap.Uint64("observed_block", blockNumber), ) - pubErr := w.verifyAndPublish(msg, ctx, eth_common.BytesToHash(msg.TxID), receipt) + verifiedMsg, verifyErr := w.verifyMessage(msg, ctx, eth_common.BytesToHash(msg.TxID), receipt) + pubErr := verifyErr + if pubErr == nil { + pubErr = w.PublishReobservation(observation, verifiedMsg) + } if pubErr != nil { w.logger.Error("Error when publishing message", zap.Error(pubErr)) @@ -166,5 +173,9 @@ func (w *Watcher) Reobserve(ctx context.Context, chainID vaa.ChainID, txID []byt } // Finally, do the reobservation and return the number of messages observed. - return w.handleReobservationRequest(ctx, chainID, txID, ethConn, finalized, safe) + validated, err := w.Validate(&gossipv1.ObservationRequest{ChainId: uint32(chainID), TxHash: txID}) + if err != nil { + return 0, err + } + return w.handleReobservationRequest(ctx, validated, ethConn, finalized, safe) } diff --git a/node/pkg/watchers/evm/watcher.go b/node/pkg/watchers/evm/watcher.go index f18416e26cf..97c8fd4aa55 100644 --- a/node/pkg/watchers/evm/watcher.go +++ b/node/pkg/watchers/evm/watcher.go @@ -83,6 +83,8 @@ var ( }, []string{"eth_network", "operation"}) ) +var _ watchers.Watcher = (*Watcher)(nil) + type ( Watcher struct { // EVM RPC url. @@ -229,6 +231,46 @@ func NewEthWatcher( } } +func (w *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, w.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + return validated, nil +} + +func (w *Watcher) ChainID() vaa.ChainID { + return w.chainID +} + +func (w *Watcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return errors.New("message publication cannot be nil") + } + + w.logger.Debug( + "publishing new message publication", + msg.ZapFields()..., + ) + w.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + ethMessagesConfirmed.WithLabelValues(w.networkName).Inc() + if msg.IsReobservation { + watchers.ReobservationsByChain.WithLabelValues(w.chainID.String(), "std").Inc() + } + + return nil +} + +func (w *Watcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return w.PublishMessage(msg) +} + func (w *Watcher) tokenBridge() eth_common.Address { var tb []byte switch w.env { @@ -422,17 +464,14 @@ func (w *Watcher) Run(parentCtx context.Context) error { case <-ctx.Done(): return nil case r := <-w.obsvReqC: - if r.ChainId > math.MaxUint16 { - logger.Error("chain id for observation request is not a valid uint16", - zap.Uint32("chainID", r.ChainId), - zap.String("txID", hex.EncodeToString(r.TxHash)), - ) + validated, err := w.Validate(r) + if err != nil { + watchers.LogInvalidObservationRequest(logger, r, err) continue } numObservations, err := w.handleReobservationRequest( ctx, - vaa.ChainID(r.ChainId), - r.TxHash, + validated, w.ethConn, atomic.LoadUint64(&w.latestFinalizedBlockNumber), atomic.LoadUint64(&w.latestSafeBlockNumber), @@ -686,7 +725,11 @@ func (w *Watcher) Run(parentCtx context.Context) error { // Note that `tx` here is actually a receipt txHash := eth_common.Hash(pLock.message.TxID) - pubErr := w.verifyAndPublish(pLock.message, ctx, txHash, tx) + verifiedMsg, verifyErr := w.verifyMessage(pLock.message, ctx, txHash, tx) + pubErr := verifyErr + if pubErr == nil { + pubErr = w.PublishMessage(verifiedMsg) + } if pubErr != nil { logger.Error("could not publish message", zap.String("msgId", pLock.message.MessageIDString()), @@ -944,7 +987,11 @@ func (w *Watcher) postMessage( verifyCtx, cancel := context.WithCancel(parentCtx) defer cancel() - pubErr := w.verifyAndPublish(msg, verifyCtx, ev.Raw.TxHash, nil) + verifiedMsg, verifyErr := w.verifyMessage(msg, verifyCtx, ev.Raw.TxHash, nil) + pubErr := verifyErr + if pubErr == nil { + pubErr = w.PublishMessage(verifiedMsg) + } if pubErr != nil { w.logger.Error("could not publish message: transfer verification failed", zap.String("msgId", msg.MessageIDString()), @@ -1007,13 +1054,9 @@ func canRetryGetBlockTime(err error) bool { return exists } -// verifyAndPublish validates a MessagePublication to ensure that it's safe. If so, it broadcasts the message. This function -// should be the only location where the watcher's msgC channel is written to. -// Modifies the verificationState field of the message as a side-effect. -// Even if an invalid Transfer is detected, the message will still be published. It is the responsibility of the calling code to handle -// a status of Rejected. -// Note that the result of verification is not returned by this function, but can be accessed directly via the reference to message. -func (w *Watcher) verifyAndPublish( +// verifyMessage applies transfer verification to a MessagePublication before it +// is sent through PublishMessage. It may modify the verification state. +func (w *Watcher) verifyMessage( // Must be non-nil and have verificationState equal to NotVerified. msg *common.MessagePublication, ctx context.Context, @@ -1022,17 +1065,17 @@ func (w *Watcher) verifyAndPublish( // This argument is only used when Transfer Verifier is enabled. If nil, transfer verifier will fetch the receipt. // Otherwise, the receipt in the calling context can be passed here to save on RPC requests and parsing. receipt *gethTypes.Receipt, -) error { +) (*common.MessagePublication, error) { if msg == nil { - return errors.New("verifyAndPublish: message publication cannot be nil") + return nil, errors.New("verifyMessage: message publication cannot be nil") } if w.txVerifier != nil { verifiedMsg, err := verify(ctx, msg, txHash, receipt, w.txVerifier) if err != nil { - return err + return nil, err } msg = &verifiedMsg w.logger.Debug( @@ -1041,17 +1084,7 @@ func (w *Watcher) verifyAndPublish( ) } - w.logger.Debug( - "publishing new message publication", - msg.ZapFields()..., - ) - w.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - ethMessagesConfirmed.WithLabelValues(w.networkName).Inc() - if msg.IsReobservation { - watchers.ReobservationsByChain.WithLabelValues(w.chainID.String(), "std").Inc() - } - return nil - + return msg, nil } // waitForBlockTime is a go routine that repeatedly attempts to read the block time for a single log event. It is used when the initial attempt to read diff --git a/node/pkg/watchers/evm/watcher_methods_test.go b/node/pkg/watchers/evm/watcher_methods_test.go new file mode 100644 index 00000000000..cd5842a13dd --- /dev/null +++ b/node/pkg/watchers/evm/watcher_methods_test.go @@ -0,0 +1,100 @@ +package evm + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" + "go.uber.org/zap" +) + +func newMethodTestWatcher(msgC chan<- *common.MessagePublication) *Watcher { + return &Watcher{chainID: vaa.ChainIDEthereum, networkName: "ethereum", msgC: msgC, logger: zap.NewNop()} +} + +func TestWatcherMethodChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDEthereum}} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newMethodTestWatcher(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestWatcherMethodValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}, wantErr: true}, + {name: "accepts empty tx hash", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum)}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherMethodPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newMethodTestWatcher(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherMethodPublishReobservation(t *testing.T) { + validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}) + require.NoError(t, err) + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newMethodTestWatcher(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/evm/watcher_test.go b/node/pkg/watchers/evm/watcher_test.go index aea2c7131a8..39aedc10a05 100644 --- a/node/pkg/watchers/evm/watcher_test.go +++ b/node/pkg/watchers/evm/watcher_test.go @@ -59,7 +59,7 @@ func Test_canRetryGetBlockTime(t *testing.T) { assert.False(t, canRetryGetBlockTime(errors.New("Hello, World!"))) } -// TestVerifyAndPublish checks the operation of the verifyAndPublish method of the watcher in +// TestVerifyAndPublish checks the operation of the verification plus PublishMessage flow in // scenarios where the Transfer Verifier is disabled and when it's enabled. It covers much of // the behaviour of the verify() function. func TestVerifyAndPublish(t *testing.T) { @@ -77,16 +77,18 @@ func TestVerifyAndPublish(t *testing.T) { require.Nil(t, w.txVerifier) // Check nil message - err := w.verifyAndPublish(nil, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err := w.verifyMessage(nil, ctx, eth_common.Hash{}, &types.Receipt{}) require.ErrorContains(t, err, "message publication cannot be nil") + require.Nil(t, verified) require.Equal(t, common.NotVerified.String(), msg.VerificationState().String()) // Check transfer verifier not enabled case. The message should be published normally. msg = common.MessagePublication{} require.Nil(t, w.txVerifier) - err = w.verifyAndPublish(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err = w.verifyMessage(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) require.NoError(t, err) + require.NoError(t, w.PublishMessage(verified)) require.Equal(t, 1, len(msgC)) publishedMsg := <-msgC require.NotNil(t, publishedMsg) @@ -105,8 +107,9 @@ func TestVerifyAndPublish(t *testing.T) { msg = common.MessagePublication{} require.Nil(t, w.txVerifier) - err = w.verifyAndPublish(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err = w.verifyMessage(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) require.NoError(t, err) + require.NoError(t, w.PublishMessage(verified)) require.Equal(t, 1, len(msgC)) publishedMsg = <-msgC require.Equal(t, common.NotVerified.String(), publishedMsg.VerificationState().String()) @@ -116,8 +119,9 @@ func TestVerifyAndPublish(t *testing.T) { msg = common.MessagePublication{} require.Nil(t, w.txVerifier) - err = w.verifyAndPublish(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err = w.verifyMessage(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) require.Nil(t, err) + require.NoError(t, w.PublishMessage(verified)) require.Equal(t, 1, len(msgC)) publishedMsg = <-msgC require.Equal(t, common.NotVerified.String(), publishedMsg.VerificationState().String()) @@ -130,8 +134,9 @@ func TestVerifyAndPublish(t *testing.T) { require.NoError(t, setErr) require.NotNil(t, w.txVerifier) - err = w.verifyAndPublish(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err = w.verifyMessage(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) require.ErrorContains(t, err, "MessagePublication already has a non-default verification state") + require.Nil(t, verified) require.Equal(t, 0, len(msgC)) require.Equal(t, common.Anomalous.String(), msg.VerificationState().String()) @@ -144,8 +149,9 @@ func TestVerifyAndPublish(t *testing.T) { EmitterAddress: tbAddr, } - err = w.verifyAndPublish(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err = w.verifyMessage(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) require.Nil(t, err) + require.NoError(t, w.PublishMessage(verified)) require.Equal(t, 1, len(msgC)) publishedMsg = <-msgC require.NotNil(t, publishedMsg) @@ -157,8 +163,9 @@ func TestVerifyAndPublish(t *testing.T) { msg = common.MessagePublication{} require.NotNil(t, w.txVerifier) - err = w.verifyAndPublish(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err = w.verifyMessage(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) require.Nil(t, err) + require.NoError(t, w.PublishMessage(verified)) require.Equal(t, 1, len(msgC)) publishedMsg = <-msgC require.Equal(t, common.NotApplicable.String(), publishedMsg.VerificationState().String()) @@ -171,8 +178,9 @@ func TestVerifyAndPublish(t *testing.T) { EmitterAddress: tbAddr, } - err = w.verifyAndPublish(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) + verified, err = w.verifyMessage(&msg, ctx, eth_common.Hash{}, &types.Receipt{}) require.NoError(t, err) + require.NoError(t, w.PublishMessage(verified)) require.Equal(t, 1, len(msgC)) publishedMsg = <-msgC require.NotNil(t, publishedMsg) diff --git a/node/pkg/watchers/ibc/watcher.go b/node/pkg/watchers/ibc/watcher.go index b6e187fe502..d5df57c128a 100644 --- a/node/pkg/watchers/ibc/watcher.go +++ b/node/pkg/watchers/ibc/watcher.go @@ -6,7 +6,6 @@ import ( "encoding/hex" "encoding/json" "fmt" - "math" "net/http" "net/url" "strconv" @@ -107,6 +106,8 @@ var ( }, []string{"ibc_channel_id"}) ) +var _ watchers.Watcher = (*chainEntry)(nil) + type ( // Watcher is responsible for monitoring the IBC contract on wormchain and publishing wormhole messages for all chains connected via IBC. Watcher struct { @@ -179,6 +180,42 @@ func NewWatcher( } } +func (ce *chainEntry) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, ce.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + return validated, nil +} + +func (ce *chainEntry) ChainID() vaa.ChainID { + return ce.chainID +} + +func (ce *chainEntry) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return fmt.Errorf("message publication is nil") + } + + ce.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + messagesConfirmed.WithLabelValues(ce.chainName).Inc() + if msg.IsReobservation { + watchers.ReobservationsByChain.WithLabelValues(msg.EmitterChain.String(), "std").Inc() + } + + return nil +} + +func (ce *chainEntry) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return ce.PublishMessage(msg) +} + // clientRequest is used to subscribe for events from the contract. type clientRequest struct { JSONRPC string `json:"jsonrpc"` @@ -354,7 +391,7 @@ func (w *Watcher) handleEvents(ctx context.Context, c *websocket.Conn) error { } if evt != nil { - if err := w.processIbcReceivePublishEvent(evt, "new"); err != nil { + if err := w.processIbcReceivePublishEvent(evt, "new", nil); err != nil { return fmt.Errorf("failed to process new IBC event: %w", err) } } @@ -436,18 +473,17 @@ func (w *Watcher) handleObservationRequests(ctx context.Context, ce *chainEntry) case <-ctx.Done(): return nil case r := <-ce.obsvReqC: - // node/pkg/node/reobserve.go already enforces the chain id is a valid uint16 - // and only writes to the channel for this chain id. - // If either of the below cases are true, something has gone wrong - if r.ChainId > math.MaxUint16 || vaa.ChainID(r.ChainId) != ce.chainID { - panic("invalid chain ID") + validated, err := ce.Validate(r) + if err != nil { + watchers.LogInvalidObservationRequest(w.logger, r, err, zap.String("chain", ce.chainName)) + continue } // SECURITY: Directly using data for URL path is scary. // Potential for directory traversal attacks to return the incorrect data // This is hex encoded so it's acceptable but be careful changing this logic. - reqTxHashStr := hex.EncodeToString(r.TxHash) - w.logger.Info("received observation request", zap.String("chain", ce.chainName), zap.String("txHash", reqTxHashStr)) + reqTxHashStr := hex.EncodeToString(validated.TxHash()) + w.logger.Info("received observation request", validated.ZapFields(zap.String("chain", ce.chainName), zap.String("txHash", reqTxHashStr))...) client := &http.Client{ Timeout: time.Second * 5, @@ -503,8 +539,7 @@ func (w *Watcher) handleObservationRequests(ctx context.Context, ce *chainEntry) } if evt != nil { - evt.Msg.IsReobservation = true - if err := w.processIbcReceivePublishEvent(evt, "reobservation"); err != nil { + if err := w.processIbcReceivePublishEvent(evt, "reobservation", &validated); err != nil { return fmt.Errorf("failed to process reobserved IBC event: %w", err) } } @@ -602,7 +637,7 @@ func parseIbcReceivePublishEvent(logger *zap.Logger, desiredContract string, eve } // processIbcReceivePublishEvent takes an IBC event, maps it to a message publication and publishes it. -func (w *Watcher) processIbcReceivePublishEvent(evt *ibcReceivePublishEvent, observationType string) error { +func (w *Watcher) processIbcReceivePublishEvent(evt *ibcReceivePublishEvent, observationType string, validated *watchers.ValidObservation) error { // SECURITY: The ibc watcher is the only watcher that can handle multiple chain IDs // To make this safe, it has a mapping from channel ID to chain IDs that it uses @@ -690,10 +725,14 @@ func (w *Watcher) processIbcReceivePublishEvent(evt *ibcReceivePublishEvent, obs zap.Uint8("ConsistencyLevel", evt.Msg.ConsistencyLevel), ) - ce.msgC <- evt.Msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - messagesConfirmed.WithLabelValues(ce.chainName).Inc() - if evt.Msg.IsReobservation { - watchers.ReobservationsByChain.WithLabelValues(evt.Msg.EmitterChain.String(), "std").Inc() + if validated != nil { + if err := ce.PublishReobservation(*validated, evt.Msg); err != nil { + return err + } + } else { + if err := ce.PublishMessage(evt.Msg); err != nil { + return err + } } return nil } diff --git a/node/pkg/watchers/ibc/watcher_methods_test.go b/node/pkg/watchers/ibc/watcher_methods_test.go new file mode 100644 index 00000000000..b855b029408 --- /dev/null +++ b/node/pkg/watchers/ibc/watcher_methods_test.go @@ -0,0 +1,104 @@ +package ibc + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newTestChainEntry(msgC chan<- *common.MessagePublication) *chainEntry { + return &chainEntry{chainID: vaa.ChainIDOsmosis, chainName: "osmosis", msgC: msgC} +} + +func TestChainEntryChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDOsmosis}} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newTestChainEntry(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestChainEntryValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDOsmosis), TxHash: make([]byte, common.TxIDLenMin)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}, wantErr: true}, + {name: "accepts empty tx hash", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDOsmosis)}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newTestChainEntry(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestChainEntryPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDOsmosis}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestChainEntry(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestChainEntryPublishReobservation(t *testing.T) { + validated, err := newTestChainEntry(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDOsmosis), TxHash: make([]byte, common.TxIDLenMin)}) + require.NoError(t, err) + + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDOsmosis}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestChainEntry(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/mock/watcher.go b/node/pkg/watchers/mock/watcher.go index 2db881c454a..e612c038e47 100644 --- a/node/pkg/watchers/mock/watcher.go +++ b/node/pkg/watchers/mock/watcher.go @@ -2,46 +2,96 @@ package mock import ( "context" + "errors" "github.com/certusone/wormhole/node/pkg/common" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/supervisor" + "github.com/certusone/wormhole/node/pkg/watchers" eth_common "github.com/ethereum/go-ethereum/common" + "github.com/wormhole-foundation/wormhole/sdk/vaa" "go.uber.org/zap" ) +type Watcher struct { + chainID vaa.ChainID + msgC chan<- *common.MessagePublication + obsvReqC <-chan *gossipv1.ObservationRequest + setC chan<- *common.GuardianSet + config *WatcherConfig +} + +var _ watchers.Watcher = (*Watcher)(nil) + +func (w *Watcher) ChainID() vaa.ChainID { + return w.chainID +} + +func (w *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + return watchers.ValidateObservationRequest(req, w.chainID) +} + +func (w *Watcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return errors.New("message publication is nil") + } + + w.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + return nil +} + +func (w *Watcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return w.PublishMessage(msg) +} + +func (w *Watcher) Run(ctx context.Context) error { + logger := supervisor.Logger(ctx) + supervisor.Signal(ctx, supervisor.SignalHealthy) + + logger.Info("Mock Watcher running.") + + for { + select { + case <-ctx.Done(): + logger.Info("Mock Watcher shutting down.") + return nil + case observation := <-w.config.MockObservationC: + logger.Info("message observed", observation.ZapFields(zap.String("digest", observation.CreateDigest()))...) + if err := w.PublishMessage(observation); err != nil { + logger.Error("failed to publish message", zap.Error(err)) + } + case gs := <-w.config.MockSetC: + w.setC <- gs //nolint:channelcheck // Will only block this mock watcher + case o := <-w.obsvReqC: + validated, err := w.Validate(o) + if err != nil { + watchers.LogInvalidObservationRequest(logger, o, err) + continue + } + hash := eth_common.BytesToHash(validated.TxHash()) + logger.Info("received observation request", validated.ZapFields(zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", hash.Hex()))...) + msg, ok := w.config.ObservationDb[hash] + if ok { + msg2 := *msg + if err := w.PublishReobservation(validated, &msg2); err != nil { + logger.Error("failed to publish reobservation", zap.Error(err)) + } + } + } + } +} + func NewWatcherRunnable( msgC chan<- *common.MessagePublication, obsvReqC <-chan *gossipv1.ObservationRequest, setC chan<- *common.GuardianSet, c *WatcherConfig, ) supervisor.Runnable { - return func(ctx context.Context) error { - logger := supervisor.Logger(ctx) - supervisor.Signal(ctx, supervisor.SignalHealthy) - - logger.Info("Mock Watcher running.") - - for { - select { - case <-ctx.Done(): - logger.Info("Mock Watcher shutting down.") - return nil - case observation := <-c.MockObservationC: - logger.Info("message observed", observation.ZapFields(zap.String("digest", observation.CreateDigest()))...) - msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - case gs := <-c.MockSetC: - setC <- gs //nolint:channelcheck // Will only block this mock watcher - case o := <-obsvReqC: - hash := eth_common.BytesToHash(o.TxHash) - logger.Info("Received obsv request", zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", hash.Hex())) - msg, ok := c.ObservationDb[hash] - if ok { - msg2 := *msg - msg2.IsReobservation = true - msgC <- &msg2 //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - } - } - } - } + w := &Watcher{chainID: c.ChainID, msgC: msgC, obsvReqC: obsvReqC, setC: setC, config: c} + return w.Run } diff --git a/node/pkg/watchers/mock/watcher_methods_test.go b/node/pkg/watchers/mock/watcher_methods_test.go new file mode 100644 index 00000000000..3b9bcb68607 --- /dev/null +++ b/node/pkg/watchers/mock/watcher_methods_test.go @@ -0,0 +1,98 @@ +package mock + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newTestWatcher(msgC chan<- *common.MessagePublication) *Watcher { + return &Watcher{chainID: vaa.ChainIDSui, msgC: msgC} +} + +func TestWatcherChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDSui}} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newTestWatcher(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestWatcherValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}, wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherPublishReobservation(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}) + require.NoError(t, err) + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/near/poll.go b/node/pkg/watchers/near/poll.go index e5b9c827e49..071682a57da 100644 --- a/node/pkg/watchers/near/poll.go +++ b/node/pkg/watchers/near/poll.go @@ -21,7 +21,7 @@ func (e *Watcher) fetchAndParseChunk(logger *zap.Logger, ctx context.Context, ch result := make([]*transactionProcessingJob, len(txns)) for i, tx := range txns { - result[i] = newTransactionProcessingJob(tx.Hash, tx.SignerId, false) + result[i] = newTransactionProcessingJob(tx.Hash, tx.SignerId, false, nil) } return result, nil } diff --git a/node/pkg/watchers/near/tx_processing.go b/node/pkg/watchers/near/tx_processing.go index 209d5c3444b..e4cc4237eab 100644 --- a/node/pkg/watchers/near/tx_processing.go +++ b/node/pkg/watchers/near/tx_processing.go @@ -12,7 +12,6 @@ import ( "time" "github.com/certusone/wormhole/node/pkg/common" - "github.com/certusone/wormhole/node/pkg/watchers" "github.com/certusone/wormhole/node/pkg/watchers/near/nearapi" eth_common "github.com/ethereum/go-ethereum/common" "github.com/mr-tron/base58" @@ -247,28 +246,23 @@ func (e *Watcher) processWormholeLog(logger *zap.Logger, _ context.Context, job IsReobservation: job.isReobservation, } - if job.isReobservation { - watchers.ReobservationsByChain.WithLabelValues("near", "std").Inc() - } - // tell everyone about it job.hasWormholeMsg = true e.eventChan <- EVENT_NEAR_MESSAGE_CONFIRMED //nolint:channelcheck // Only pauses this watcher - logger.Info("message observed", - zap.String("log_msg_type", "wormhole_event_success"), - zap.Uint64("ts", ts), - zap.Time("timestamp", observation.Timestamp), - zap.Uint32("nonce", observation.Nonce), - zap.Uint64("sequence", observation.Sequence), - zap.Stringer("emitter_chain", observation.EmitterChain), - zap.Stringer("emitter_address", observation.EmitterAddress), - zap.Binary("payload", observation.Payload), - zap.Uint8("consistency_level", observation.ConsistencyLevel), - ) - - e.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + logger.Info("message observed", observation.ZapFields(zap.String("log_msg_type", "wormhole_event_success"), zap.Uint64("ts", ts))...) + + if job.validated != nil { + if err := e.PublishReobservation(*job.validated, observation); err != nil { + return err + } + return nil + } + + if err := e.PublishMessage(observation); err != nil { + return err + } return nil } diff --git a/node/pkg/watchers/near/watcher.go b/node/pkg/watchers/near/watcher.go index ec4663949f6..1030db2e0f8 100644 --- a/node/pkg/watchers/near/watcher.go +++ b/node/pkg/watchers/near/watcher.go @@ -11,6 +11,7 @@ import ( gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" "github.com/certusone/wormhole/node/pkg/readiness" "github.com/certusone/wormhole/node/pkg/supervisor" + "github.com/certusone/wormhole/node/pkg/watchers" "github.com/certusone/wormhole/node/pkg/watchers/near/nearapi" "github.com/ethereum/go-ethereum/common/math" "github.com/mr-tron/base58" @@ -50,6 +51,8 @@ var ( nearBlockchainMaxGaps = 5 ) +var _ watchers.Watcher = (*Watcher)(nil) + type ( transactionProcessingJob struct { txHash string @@ -58,12 +61,14 @@ type ( retryCounter uint delay time.Duration isReobservation bool + validated *watchers.ValidObservation // set during processing hasWormholeMsg bool // set during processing; whether this transaction emitted a Wormhole message } Watcher struct { + chainID vaa.ChainID mainnet bool wormholeAccount string // name of the Wormhole Account on the NEAR blockchain nearRPC string @@ -100,6 +105,7 @@ func NewWatcher( mainnet bool, ) *Watcher { return &Watcher{ + chainID: vaa.ChainIDNear, mainnet: mainnet, wormholeAccount: wormholeContract, nearRPC: nearRPC, @@ -113,7 +119,7 @@ func NewWatcher( } } -func newTransactionProcessingJob(txHash string, senderAccountId string, isReobservation bool) *transactionProcessingJob { +func newTransactionProcessingJob(txHash string, senderAccountId string, isReobservation bool, validated *watchers.ValidObservation) *transactionProcessingJob { return &transactionProcessingJob{ txHash, senderAccountId, @@ -121,10 +127,46 @@ func newTransactionProcessingJob(txHash string, senderAccountId string, isReobse 0, initialTxProcDelay, isReobservation, + validated, false, } } +func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, e.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + return validated, nil +} + +func (e *Watcher) ChainID() vaa.ChainID { + return e.chainID +} + +func (e *Watcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return fmt.Errorf("message publication is nil") + } + + e.msgC <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + if msg.IsReobservation { + watchers.ReobservationsByChain.WithLabelValues(e.chainID.String(), "std").Inc() + } + + return nil +} + +func (e *Watcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return e.PublishMessage(msg) +} + func (e *Watcher) runBlockPoll(ctx context.Context) error { logger := supervisor.Logger(ctx) @@ -200,23 +242,22 @@ func (e *Watcher) runObsvReqProcessor(ctx context.Context) error { case <-ctx.Done(): return ctx.Err() case r := <-e.obsvReqC: - // node/pkg/node/reobserve.go already enforces the chain id is a valid uint16 - // and only writes to the channel for this chain id. - // If either of the below cases are true, something has gone wrong - if r.ChainId > math.MaxUint16 || vaa.ChainID(r.ChainId) != vaa.ChainIDNear { - panic("invalid chain ID") + validated, err := e.Validate(r) + if err != nil { + watchers.LogInvalidObservationRequest(logger, r, err, zap.String("txIDBase58", base58.Encode(r.GetTxHash()))) + continue } - txHash := base58.Encode(r.TxHash) + txHash := base58.Encode(validated.TxHash()) - logger.Info("Received obsv request", zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", txHash)) + logger.Info("received observation request", validated.ZapFields(zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", txHash))...) // TODO e.wormholeContract is not the correct value for senderAccountId. Instead, it should be the account id of the transaction sender. // This value is used by NEAR to determine which shard to query. An incorrect value here is not a security risk but could lead to reobservation requests failing. // Guardians currently run nodes for all shards and the API seems to be returning the correct results independent of the set senderAccountId but this could change in the future. // Fixing this would require adding the transaction sender account ID to the observation request. - job := newTransactionProcessingJob(txHash, e.wormholeAccount, true) - err := e.schedule(ctx, job, time.Nanosecond) + job := newTransactionProcessingJob(txHash, e.wormholeAccount, true, &validated) + err = e.schedule(ctx, job, time.Nanosecond) if err != nil { // Error-level logging here because this is after an re-observation request already, which should be infrequent logger.Error("error scheduling transaction processing job", zap.Error(err)) diff --git a/node/pkg/watchers/near/watcher_methods_test.go b/node/pkg/watchers/near/watcher_methods_test.go new file mode 100644 index 00000000000..192a3d29330 --- /dev/null +++ b/node/pkg/watchers/near/watcher_methods_test.go @@ -0,0 +1,99 @@ +package near + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newTestWatcher(msgC chan<- *common.MessagePublication) *Watcher { + return &Watcher{chainID: vaa.ChainIDNear, msgC: msgC} +} + +func TestWatcherChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDNear}} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newTestWatcher(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestWatcherValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDNear), TxHash: make([]byte, common.TxIDLenMin)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}, wantErr: true}, + {name: "accepts empty tx hash", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDNear)}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDNear}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherPublishReobservation(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDNear), TxHash: make([]byte, common.TxIDLenMin)}) + require.NoError(t, err) + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDNear}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/observation.go b/node/pkg/watchers/observation.go new file mode 100644 index 00000000000..76e131057d7 --- /dev/null +++ b/node/pkg/watchers/observation.go @@ -0,0 +1,110 @@ +package watchers + +import ( + "encoding/hex" + "errors" + "fmt" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/wormhole-foundation/wormhole/sdk/vaa" + "go.uber.org/zap" +) + +// LogInvalidObservationRequest logs a validation failure for a raw gossiped +// observation request using a consistent set of structured fields. +func LogInvalidObservationRequest(logger *zap.Logger, req *gossipv1.ObservationRequest, err error, fields ...zap.Field) { + fields = append(fields, zap.Error(err)) + if req == nil { + logger.Error("invalid observation request", append(fields, zap.Bool("nilRequest", true))...) + return + } + + logger.Error("invalid observation request", append(fields, + zap.Uint32("chainID", req.GetChainId()), + zap.String("txID", hex.EncodeToString(req.GetTxHash())), + zap.Int64("timestamp", req.GetTimestamp()), + )...) +} + +// ValidObservation is a watcher-scoped observation request that has passed the +// fundamental chain and request-shape validation checks. +type ValidObservation struct { + chainID vaa.ChainID + txHash []byte + timestamp int64 +} + +// ValidateObservationRequest validates a raw observation request against the +// expected watcher chain and returns a copy-on-read validated value. +func ValidateObservationRequest(req *gossipv1.ObservationRequest, expectedChainID vaa.ChainID) (ValidObservation, error) { + if req == nil { + return ValidObservation{}, errors.New("observation request is nil") + } + + chainID, err := vaa.KnownChainIDFromNumber(req.ChainId) + if err != nil { + return ValidObservation{}, fmt.Errorf("invalid chain id %d: %w", req.ChainId, err) + } + + if chainID != expectedChainID { + return ValidObservation{}, fmt.Errorf("unexpected chain id %v, expected %v", chainID, expectedChainID) + } + + return ValidObservation{ + chainID: chainID, + txHash: append([]byte(nil), req.TxHash...), + timestamp: req.Timestamp, + }, nil +} + +// ChainID returns the validated chain ID. +func (v ValidObservation) ChainID() vaa.ChainID { + return v.chainID +} + +// TxHash returns a defensive copy of the validated transaction hash bytes. +func (v ValidObservation) TxHash() []byte { + return append([]byte(nil), v.txHash...) +} + +// Timestamp returns the validated wire timestamp. +func (v ValidObservation) Timestamp() int64 { + return v.timestamp +} + +// ZapFields takes some zap fields and appends zap fields related to the +// validated observation request. +func (v ValidObservation) ZapFields(fields ...zap.Field) []zap.Field { + return append(fields, + zap.Uint32("chainID", uint32(v.chainID)), + zap.String("chain", v.chainID.String()), + zap.String("txID", hex.EncodeToString(v.txHash)), + zap.Int64("timestamp", v.timestamp), + ) +} + +// RequireTxHashLength enforces watcher-specific transaction identifier sizes. +func (v ValidObservation) RequireTxHashLength(lengths ...int) error { + for _, length := range lengths { + if len(v.txHash) == length { + return nil + } + } + + return fmt.Errorf("unexpected tx hash length %d", len(v.txHash)) +} + +// ValidateReobservedMessage ensures that a message publication is safe to +// publish as a reobservation for the validated watcher chain. +func ValidateReobservedMessage(observation ValidObservation, msg *common.MessagePublication) error { + if msg == nil { + return errors.New("message publication is nil") + } + + if msg.EmitterChain != observation.chainID { + return fmt.Errorf("message publication emitter chain %v does not match validated observation chain %v", msg.EmitterChain, observation.chainID) + } + + return nil +} diff --git a/node/pkg/watchers/observation_test.go b/node/pkg/watchers/observation_test.go new file mode 100644 index 00000000000..29fa8e2d886 --- /dev/null +++ b/node/pkg/watchers/observation_test.go @@ -0,0 +1,135 @@ +package watchers + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +func TestValidateObservationRequest(t *testing.T) { + t.Run("rejects nil request", func(t *testing.T) { + _, err := ValidateObservationRequest(nil, vaa.ChainIDSui) + require.Error(t, err) + assert.Contains(t, err.Error(), "nil") + }) + + t.Run("accepts expected chain", func(t *testing.T) { + validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + ChainId: uint32(vaa.ChainIDSui), + TxHash: []byte{1, 2, 3}, + Timestamp: 1234, + }, vaa.ChainIDSui) + require.NoError(t, err) + assert.Equal(t, vaa.ChainIDSui, validated.ChainID()) + assert.Equal(t, []byte{1, 2, 3}, validated.TxHash()) + assert.Equal(t, int64(1234), validated.Timestamp()) + + original := validated.TxHash() + original[0] = 99 + assert.Equal(t, []byte{1, 2, 3}, validated.TxHash()) + }) + + t.Run("rejects unknown chain number", func(t *testing.T) { + _, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + ChainId: 999999, + TxHash: []byte{1, 2, 3}, + }, vaa.ChainIDSui) + require.Error(t, err) + assert.Contains(t, err.Error(), "invalid chain id") + }) + + t.Run("rejects unexpected chain", func(t *testing.T) { + _, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + ChainId: uint32(vaa.ChainIDAptos), + TxHash: []byte{1, 2, 3}, + }, vaa.ChainIDSui) + require.Error(t, err) + assert.Contains(t, err.Error(), "unexpected chain id") + }) + + t.Run("tx hash length helper", func(t *testing.T) { + validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + ChainId: uint32(vaa.ChainIDSui), + TxHash: make([]byte, common.TxIDLenMin), + }, vaa.ChainIDSui) + require.NoError(t, err) + require.NoError(t, validated.RequireTxHashLength(common.TxIDLenMin)) + require.Error(t, validated.RequireTxHashLength(64)) + }) +} + +func TestValidateReobservedMessage(t *testing.T) { + validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + ChainId: uint32(vaa.ChainIDSui), + TxHash: make([]byte, common.TxIDLenMin), + }, vaa.ChainIDSui) + require.NoError(t, err) + + t.Run("rejects nil message", func(t *testing.T) { + require.Error(t, ValidateReobservedMessage(validated, nil)) + }) + + t.Run("rejects mismatched chain", func(t *testing.T) { + msg := &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum} + require.Error(t, ValidateReobservedMessage(validated, msg)) + }) + + msg := &common.MessagePublication{EmitterChain: vaa.ChainIDSui} + require.NoError(t, ValidateReobservedMessage(validated, msg)) +} + +func TestValidObservationZapFields(t *testing.T) { + validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + ChainId: uint32(vaa.ChainIDSui), + TxHash: []byte{0xAA, 0xBB}, + Timestamp: 42, + }, vaa.ChainIDSui) + require.NoError(t, err) + + fields := validated.ZapFields(zap.String("extra", "value")) + require.Len(t, fields, 5) + assert.Equal(t, zap.String("extra", "value").Key, fields[0].Key) + assert.Equal(t, "chainID", fields[1].Key) + assert.Equal(t, "chain", fields[2].Key) + assert.Equal(t, "txID", fields[3].Key) + assert.Equal(t, "timestamp", fields[4].Key) +} + +func TestLogInvalidObservationRequest(t *testing.T) { + t.Run("logs raw request fields", func(t *testing.T) { + core, logs := observer.New(zap.ErrorLevel) + logger := zap.New(core) + req := &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: []byte{0xAA}, Timestamp: 42} + err := assert.AnError + + LogInvalidObservationRequest(logger, req, err, zap.String("extra", "value")) + + entries := logs.AllUntimed() + require.Len(t, entries, 1) + assert.Equal(t, "invalid observation request", entries[0].Message) + ctx := entries[0].ContextMap() + assert.Equal(t, uint32(vaa.ChainIDSui), ctx["chainID"]) + assert.Equal(t, "aa", ctx["txID"]) + assert.Equal(t, int64(42), ctx["timestamp"]) + assert.Equal(t, "value", ctx["extra"]) + require.Contains(t, ctx, "error") + }) + + t.Run("logs nil request", func(t *testing.T) { + core, logs := observer.New(zap.ErrorLevel) + logger := zap.New(core) + + LogInvalidObservationRequest(logger, nil, assert.AnError) + + entries := logs.AllUntimed() + require.Len(t, entries, 1) + ctx := entries[0].ContextMap() + assert.Equal(t, true, ctx["nilRequest"]) + }) +} diff --git a/node/pkg/watchers/solana/client.go b/node/pkg/watchers/solana/client.go index 45928778a5c..f8c4e63ad4c 100644 --- a/node/pkg/watchers/solana/client.go +++ b/node/pkg/watchers/solana/client.go @@ -5,14 +5,12 @@ import ( "context" "errors" "fmt" - "math" "strings" "sync" "sync/atomic" "time" "encoding/base64" - "encoding/hex" "encoding/json" "github.com/certusone/wormhole/node/pkg/common" @@ -235,6 +233,8 @@ const ( accountPrefixUnreliable = "msu" ) +var _ watchers.Watcher = (*SolanaWatcher)(nil) + // PostMessageData represents the user-supplied, untrusted instruction data // for message publications. We use this to determine consistency level before fetching accounts. type PostMessageData struct { @@ -285,6 +285,49 @@ func NewSolanaWatcher( } } +func (s *SolanaWatcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, s.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + // Solana reobservations accept either a message account public key or a + // transaction signature, so both wire lengths are valid here. + if err := validated.RequireTxHashLength(SolanaAccountLen, SolanaSignatureLen); err != nil { + return watchers.ValidObservation{}, err + } + + return validated, nil +} + +func (s *SolanaWatcher) ChainID() vaa.ChainID { + return s.chainID +} + +func (s *SolanaWatcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return errors.New("message publication is nil") + } + + s.msgC <- msg // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + solanaMessagesConfirmed.WithLabelValues(s.networkName).Inc() + return nil +} + +func (s *SolanaWatcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + if err := s.PublishMessage(msg); err != nil { + return err + } + + watchers.ReobservationsByChain.WithLabelValues(s.chainID.String(), "std").Inc() + return nil +} + func (s *SolanaWatcher) setupSubscription(ctx context.Context, logger *zap.Logger) (*websocket.Conn, error) { logger.Info(fmt.Sprintf("%s watcher connecting to WS node ", s.chainID.String()), zap.String("url", s.wsUrl)) @@ -425,16 +468,14 @@ func (s *SolanaWatcher) Run(ctx context.Context) error { return err } case m := <-s.obsvReqC: - if m.ChainId > math.MaxUint16 { - logger.Error("chain id for observation request is not a valid uint16", - zap.Uint32("chainID", m.ChainId), - zap.String("txID", hex.EncodeToString(m.TxHash)), - ) + validated, err := s.Validate(m) + if err != nil { + watchers.LogInvalidObservationRequest(logger, m, err) continue } //nolint:contextcheck // Passed via the 's' object instead of as a parameter. - numObservations, err := s.handleReobservationRequest(vaa.ChainID(m.ChainId), m.TxHash, s.rpcClient) + numObservations, err := s.handleReobservationRequest(validated, s.rpcClient) if err != nil { logger.Error("failed to process observation request", zap.Uint32("chainID", m.ChainId), @@ -643,7 +684,7 @@ func (s *SolanaWatcher) fetchBlock(ctx context.Context, logger *zap.Logger, slot continue } - s.processTransaction(ctx, s.rpcClient, tx, txRpc.Meta, slot, false) + s.processTransaction(ctx, s.rpcClient, tx, txRpc.Meta, slot, nil) } if emptyRetry > 0 && logger.Level().Enabled(zapcore.DebugLevel) { @@ -657,7 +698,8 @@ func (s *SolanaWatcher) fetchBlock(ctx context.Context, logger *zap.Logger, slot } // processTransaction processes a transaction and publishes any Wormhole events. -func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.Client, tx *solana.Transaction, meta *rpc.TransactionMeta, slot uint64, isReobservation bool) (numObservations uint32) { +func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.Client, tx *solana.Transaction, meta *rpc.TransactionMeta, slot uint64, validated *watchers.ValidObservation) (numObservations uint32) { + isReobservation := validated != nil // SECURITY: Validate transaction metadata before accessing fields if metadataErr := validateTransactionMeta(meta); metadataErr != nil { if s.logger.Level().Enabled(zapcore.DebugLevel) { @@ -680,23 +722,27 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C return } - var programIndex uint16 - var shimProgramIndex uint16 - var shimFound bool + var ( + programIndex uint16 + shimProgramIndex uint16 + shimFound bool + ) // SECURITY: Mapping of AccountKeys matches the indexes associated with the original transaction. // This is filled via a helper function. The ordering is AccountKeys (static accounts), Writable Account Lookup Table (ALT) entries, and Readable ALT entries. - // - for n, key := range tx.Message.AccountKeys { + for i, key := range tx.Message.AccountKeys { + index := uint16(i) // #nosec G115 -- The solana runtime can only support 64 accounts per transaction max if key.Equals(s.contract) { - programIndex = uint16(n) // #nosec G115 -- The solana runtime can only support 64 accounts per transaction max + programIndex = index } if s.shimEnabled && key.Equals(s.shimContractAddr) { - shimProgramIndex = uint16(n) // #nosec G115 -- The solana runtime can only support 64 accounts per transaction max + shimProgramIndex = index shimFound = true } } + if programIndex == 0 { + // No account keys in the message or the shim program's public key was not found among them. return } @@ -731,7 +777,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C } } } else { - found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, i, isReobservation) + found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, i, validated) if err != nil { s.logger.Error("malformed Wormhole instruction", zap.Error(err), @@ -777,7 +823,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C } } } else { - found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, innerIdx, isReobservation) + found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, innerIdx, validated) if err != nil { s.logger.Error("malformed Wormhole instruction", zap.Error(err), @@ -805,7 +851,8 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C return } -func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.Client, slot uint64, inst solana.CompiledInstruction, programIndex uint16, tx *solana.Transaction, signature solana.Signature, idx int, isReobservation bool) (bool, error) { +func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.Client, slot uint64, inst solana.CompiledInstruction, programIndex uint16, tx *solana.Transaction, signature solana.Signature, idx int, validated *watchers.ValidObservation) (bool, error) { + isReobservation := validated != nil if inst.ProgramIDIndex != programIndex { return false, nil } @@ -859,15 +906,15 @@ func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.C } common.RunWithScissors(ctx, s.errC, "retryFetchMessageAccount", func(ctx context.Context) error { - s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, 0, isReobservation, signature) + s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, 0, validated, signature) return nil }) return true, nil } -func (s *SolanaWatcher) retryFetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, retry uint, isReobservation bool, signature solana.Signature) { - _, retryable := s.fetchMessageAccount(ctx, rpcClient, acc, slot, isReobservation, signature) +func (s *SolanaWatcher) retryFetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, retry uint, validated *watchers.ValidObservation, signature solana.Signature) { + _, retryable := s.fetchMessageAccount(ctx, rpcClient, acc, slot, validated, signature) if retryable { if retry >= maxRetries { @@ -886,13 +933,13 @@ func (s *SolanaWatcher) retryFetchMessageAccount(ctx context.Context, rpcClient zap.Uint("retry", retry)) common.RunWithScissors(ctx, s.errC, "retryFetchMessageAccount", func(ctx context.Context) error { - s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, retry+1, isReobservation, signature) + s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, retry+1, validated, signature) return nil }) } } -func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, isReobservation bool, signature solana.Signature) (numObservations uint32, retryable bool) { +func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, validated *watchers.ValidObservation, signature solana.Signature) (numObservations uint32, retryable bool) { // Fetching account rCtx, cancel := context.WithTimeout(ctx, rpcTimeout) defer cancel() @@ -948,7 +995,7 @@ func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc. zap.Binary("data", data)) } - return s.processMessageAccount(s.logger, data, acc, isReobservation, signature), false + return s.processMessageAccount(s.logger, data, acc, validated, signature), false } func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger *zap.Logger, data []byte, isReobservation bool) error { @@ -1002,7 +1049,7 @@ func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger switch string(data[:3]) { case accountPrefixReliable, accountPrefixUnreliable: acc := solana.PublicKeyFromBytes([]byte(value.Pubkey)) - s.processMessageAccount(logger, data, acc, isReobservation, solana.Signature{}) + s.processMessageAccount(logger, data, acc, nil, solana.Signature{}) default: break } @@ -1011,7 +1058,8 @@ func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger } // SECURITY: Ownership check on account key must be done BEFORE this function is called. -func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, acc solana.PublicKey, isReobservation bool, signature solana.Signature) (numObservations uint32) { +func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, acc solana.PublicKey, validated *watchers.ValidObservation, signature solana.Signature) (numObservations uint32) { + isReobservation := validated != nil proposal, err := ParseMessagePublicationAccount(data) if err != nil { solanaAccountSkips.WithLabelValues(s.networkName, "parse_transfer_out").Inc() @@ -1086,41 +1134,26 @@ func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, a // SECURITY: An unreliable message with an empty payload is most like a PostMessage generated as part // of a shim event where this guardian is not watching the shim contract. Those events should be ignored. if !reliable && len(observation.Payload) == 0 { - logger.Debug("ignoring an observation because it is marked unreliable and has a zero length payload, probably from the shim", - zap.Stringer("account", acc), - zap.Time("timestamp", observation.Timestamp), - zap.Uint32("nonce", observation.Nonce), - zap.Uint64("sequence", observation.Sequence), - zap.Stringer("emitter_chain", observation.EmitterChain), - zap.Stringer("emitter_address", observation.EmitterAddress), - zap.Bool("isReobservation", isReobservation), - zap.Binary("payload", observation.Payload), - zap.Uint8("consistency_level", observation.ConsistencyLevel), - ) + logger.Debug("ignoring an observation because it is marked unreliable and has a zero length payload, probably from the shim", observation.ZapFields(zap.Stringer("account", acc))...) return } - solanaMessagesConfirmed.WithLabelValues(s.networkName).Inc() - if isReobservation { - watchers.ReobservationsByChain.WithLabelValues(s.chainID.String(), "std").Inc() + if logger.Level().Enabled(s.msgObservedLogLevel) { + logger.Log(s.msgObservedLogLevel, "message observed", observation.ZapFields(zap.Stringer("account", acc), zap.Stringer("signature", signature))...) } - if logger.Level().Enabled(s.msgObservedLogLevel) { - logger.Log(s.msgObservedLogLevel, "message observed", - zap.Stringer("account", acc), - zap.Stringer("signature", signature), - zap.Time("timestamp", observation.Timestamp), - zap.Uint32("nonce", observation.Nonce), - zap.Uint64("sequence", observation.Sequence), - zap.Stringer("emitter_chain", observation.EmitterChain), - zap.Stringer("emitter_address", observation.EmitterAddress), - zap.Bool("isReobservation", isReobservation), - zap.Binary("payload", observation.Payload), - zap.Uint8("consistency_level", observation.ConsistencyLevel), - ) + if validated != nil { + if err := s.PublishReobservation(*validated, observation); err != nil { + logger.Error("failed to publish reobservation", zap.Error(err)) + return 0 + } + return 1 } - s.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + if err := s.PublishMessage(observation); err != nil { + logger.Error("failed to publish message", zap.Error(err)) + return 0 + } return 1 } diff --git a/node/pkg/watchers/solana/client_test.go b/node/pkg/watchers/solana/client_test.go index 779218399c1..c12a7f74952 100644 --- a/node/pkg/watchers/solana/client_test.go +++ b/node/pkg/watchers/solana/client_test.go @@ -14,6 +14,8 @@ import ( "time" "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/certusone/wormhole/node/pkg/watchers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -412,7 +414,13 @@ func TestProcessMessageAccount(t *testing.T) { data := encodeMessagePublicationAccount(t, tc.prefix, proposal) acc := solana.PublicKeyFromBytes(bytes.Repeat([]byte{0x11}, solana.PublicKeyLength)) - num := s.processMessageAccount(s.logger, data, acc, tc.isReobservation, solana.Signature{}) + var validated *watchers.ValidObservation + if tc.isReobservation { + obs, err := s.Validate(&gossipv1.ObservationRequest{ChainId: uint32(tc.chainID), TxHash: acc.Bytes()}) + require.NoError(t, err) + validated = &obs + } + num := s.processMessageAccount(s.logger, data, acc, validated, solana.Signature{}) assert.Equal(t, tc.wantCount, num) if tc.wantCount == 0 { @@ -599,7 +607,7 @@ func TestProcessInstructionEarlyReturns(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - found, err := s.processInstruction(context.TODO(), nil, 1, tc.inst, 0, tx, signature, 0, false) + found, err := s.processInstruction(context.TODO(), nil, 1, tc.inst, 0, tx, signature, 0, nil) if tc.wantErr { require.Error(t, err) return @@ -663,7 +671,7 @@ func TestProcessInstructionValidPostMessage(t *testing.T) { Accounts: []uint16{0, 1, 0, 0, 0, 0, 0, 0}, } - found, err := s.processInstruction(context.Background(), rpcClient, 1, inst, 0, tx, tx.Signatures[0], 0, false) + found, err := s.processInstruction(context.Background(), rpcClient, 1, inst, 0, tx, tx.Signatures[0], 0, nil) require.NoError(t, err) assert.True(t, found) @@ -876,7 +884,7 @@ func TestProcessTransaction(t *testing.T) { InnerInstructions: tc.innerInstructions, } - num := s.processTransaction(context.Background(), rpcClient, tx, meta, 42, false) + num := s.processTransaction(context.Background(), rpcClient, tx, meta, 42, nil) assert.Equal(t, tc.wantObservations, num) // Drain published messages and verify count. diff --git a/node/pkg/watchers/solana/reobserve.go b/node/pkg/watchers/solana/reobserve.go index 3644c66e218..c65b4792a91 100644 --- a/node/pkg/watchers/solana/reobserve.go +++ b/node/pkg/watchers/solana/reobserve.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/certusone/wormhole/node/pkg/watchers" "github.com/gagliardetto/solana-go" "github.com/gagliardetto/solana-go/rpc" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -12,19 +14,17 @@ import ( // handleReobservationRequest performs a reobservation request and publishes any observed transactions. // SECURITY: Only the finalized watcher handles reobservations. Set in configuration of watcher for Solana chains. -func (s *SolanaWatcher) handleReobservationRequest(chainId vaa.ChainID, txID []byte, rpcClient *rpc.Client) (numObservations uint32, err error) { - if chainId != s.chainID { - return 0, fmt.Errorf("unexpected chain id: %v", chainId) - } +func (s *SolanaWatcher) handleReobservationRequest(observation watchers.ValidObservation, rpcClient *rpc.Client) (numObservations uint32, err error) { + txID := observation.TxHash() if len(txID) == SolanaAccountLen { // Request by account ID acc := solana.PublicKeyFromBytes(txID) - s.logger.Info("received observation request with account id", zap.String("account", acc.String())) + s.logger.Info("received observation request with account id", observation.ZapFields(zap.String("account", acc.String()))...) rCtx, cancel := context.WithTimeout(s.ctx, rpcTimeout) - numObservations, _ = s.fetchMessageAccount(rCtx, rpcClient, acc, 0, true, solana.Signature{}) + numObservations, _ = s.fetchMessageAccount(rCtx, rpcClient, acc, 0, &observation, solana.Signature{}) cancel() } else if len(txID) == SolanaSignatureLen { // Request by transaction ID signature := solana.SignatureFromBytes(txID) - s.logger.Info("received observation request with transaction id", zap.Stringer("signature", signature)) + s.logger.Info("received observation request with transaction id", observation.ZapFields(zap.Stringer("signature", signature))...) rCtx, cancel := context.WithTimeout(s.ctx, rpcTimeout) version := uint64(0) result, err := rpcClient.GetTransaction( @@ -52,7 +52,7 @@ func (s *SolanaWatcher) handleReobservationRequest(chainId vaa.ChainID, txID []b if err != nil { return 0, fmt.Errorf("failed to extract transaction for observation request: %v", err) } - numObservations = s.processTransaction(s.ctx, rpcClient, tx, result.Meta, result.Slot, true) + numObservations = s.processTransaction(s.ctx, rpcClient, tx, result.Meta, result.Slot, &observation) } else { return 0, fmt.Errorf("ignoring an observation request of unexpected length: %d", len(txID)) } @@ -67,5 +67,9 @@ func (s *SolanaWatcher) Reobserve(_ context.Context, chainID vaa.ChainID, txID [ s.logger.Info("received a request to reobserve using a custom endpoint", zap.Stringer("chainID", chainID), zap.Any("txID", txID), zap.String("url", customEndpoint)) rpcClient := rpc.New(customEndpoint) //nolint:contextcheck // See comment above for the reason why we don't use the passed in context. - return s.handleReobservationRequest(chainID, txID, rpcClient) + validated, err := s.Validate(&gossipv1.ObservationRequest{ChainId: uint32(chainID), TxHash: txID}) + if err != nil { + return 0, err + } + return s.handleReobservationRequest(validated, rpcClient) } diff --git a/node/pkg/watchers/solana/shim.go b/node/pkg/watchers/solana/shim.go index f6a5dcd7eac..850e5c30a81 100644 --- a/node/pkg/watchers/solana/shim.go +++ b/node/pkg/watchers/solana/shim.go @@ -373,26 +373,16 @@ func (s *SolanaWatcher) shimProcessRest( Unreliable: false, } - solanaMessagesConfirmed.WithLabelValues(s.networkName).Inc() + if logger.Level().Enabled(s.msgObservedLogLevel) { + logger.Log(s.msgObservedLogLevel, "message observed from shim", observation.ZapFields(zap.Stringer("signature", tx.Signatures[0]))...) + } + + if err := s.PublishMessage(observation); err != nil { + return err + } if isReobservation { watchers.ReobservationsByChain.WithLabelValues(s.chainID.String(), "shim").Inc() } - if logger.Level().Enabled(s.msgObservedLogLevel) { - logger.Log(s.msgObservedLogLevel, "message observed from shim", - zap.Stringer("signature", tx.Signatures[0]), - zap.Time("timestamp", observation.Timestamp), - zap.Uint32("nonce", observation.Nonce), - zap.Uint64("sequence", observation.Sequence), - zap.Stringer("emitter_chain", observation.EmitterChain), - zap.Stringer("emitter_address", observation.EmitterAddress), - zap.Bool("isReobservation", observation.IsReobservation), - zap.Binary("payload", observation.Payload), - zap.Uint8("consistency_level", observation.ConsistencyLevel), - ) - } - - s.msgC <- observation //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - return nil } diff --git a/node/pkg/watchers/solana/tx_for_addr.go b/node/pkg/watchers/solana/tx_for_addr.go index 6de893dba67..5a718710d12 100644 --- a/node/pkg/watchers/solana/tx_for_addr.go +++ b/node/pkg/watchers/solana/tx_for_addr.go @@ -193,7 +193,7 @@ func (s *SolanaWatcher) processTransactionWithRetry(signature solana.Signature) return } - _ = s.processTransaction(s.ctx, s.rpcClient, tx, result.Meta, result.Slot, false) + _ = s.processTransaction(s.ctx, s.rpcClient, tx, result.Meta, result.Slot, nil) return } diff --git a/node/pkg/watchers/solana/watcher_methods_test.go b/node/pkg/watchers/solana/watcher_methods_test.go new file mode 100644 index 00000000000..cc1d834b6b2 --- /dev/null +++ b/node/pkg/watchers/solana/watcher_methods_test.go @@ -0,0 +1,100 @@ +package solana + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newMethodTestWatcher(msgC chan<- *common.MessagePublication) *SolanaWatcher { + return &SolanaWatcher{chainID: vaa.ChainIDSolana, networkName: "solana", msgC: msgC} +} + +func TestWatcherChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDSolana}} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newMethodTestWatcher(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestWatcherValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts account length", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSolana), TxHash: make([]byte, SolanaAccountLen)}}, + {name: "accepts signature length", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSolana), TxHash: make([]byte, SolanaSignatureLen)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, SolanaAccountLen)}, wantErr: true}, + {name: "rejects unsupported length", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSolana), TxHash: make([]byte, 10)}, wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSolana}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newMethodTestWatcher(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherPublishReobservation(t *testing.T) { + validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSolana), TxHash: make([]byte, SolanaAccountLen)}) + require.NoError(t, err) + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSolana}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newMethodTestWatcher(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/sui/watcher.go b/node/pkg/watchers/sui/watcher.go index 6964bee250a..9554a159a47 100644 --- a/node/pkg/watchers/sui/watcher.go +++ b/node/pkg/watchers/sui/watcher.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "math" "net/http" "strconv" "strings" @@ -35,6 +34,7 @@ import ( type ( // Watcher is responsible for looking over Sui blockchain and reporting new transactions to the wormhole contract Watcher struct { + chainID vaa.ChainID suiRPC string suiMoveEventType string @@ -183,6 +183,8 @@ var ( }) ) +var _ watchers.Watcher = (*Watcher)(nil) + // NewWatcher creates a new Sui appid watcher func NewWatcher( suiRPC string, @@ -246,6 +248,7 @@ func NewWatcher( } return &Watcher{ + chainID: vaa.ChainIDSui, suiRPC: suiRPC, suiMoveEventType: suiMoveEventType, unsafeDevMode: unsafeDevMode, @@ -264,7 +267,44 @@ func NewWatcher( }, nil } -func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiResult, isReobservation bool) error { +func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { + validated, err := watchers.ValidateObservationRequest(req, e.chainID) + if err != nil { + return watchers.ValidObservation{}, err + } + + return validated, nil +} + +func (e *Watcher) ChainID() vaa.ChainID { + return e.chainID +} + +func (e *Watcher) PublishMessage(msg *common.MessagePublication) error { + if msg == nil { + return errors.New("message publication is nil") + } + + e.msgChan <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations + suiMessagesConfirmed.Inc() + if msg.IsReobservation { + watchers.ReobservationsByChain.WithLabelValues(e.chainID.String(), "std").Inc() + } + + return nil +} + +func (e *Watcher) PublishReobservation(observation watchers.ValidObservation, msg *common.MessagePublication) error { + if err := watchers.ValidateReobservedMessage(observation, msg); err != nil { + return err + } + + msg.IsReobservation = true + return e.PublishMessage(msg) +} + +func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiResult, validated *watchers.ValidObservation) error { + isReobservation := validated != nil if body.ID.TxDigest == nil { return errors.New("missing TxDigest field") } @@ -339,7 +379,40 @@ func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiR // Verifies the observation through the Sui transaction verifier, if enabled, followed // by publishing the observation to the message channel. - err = e.verifyAndPublish(ctx, observation, *body.ID.TxDigest, logger) + if validated != nil { + if e.suiTxVerifier != nil { + verifiedMsg, verifyErr := e.verify(ctx, observation, *body.ID.TxDigest, logger) + if verifyErr != nil { + err = verifyErr + } else { + observation = &verifiedMsg + } + } + + if err == nil { + err = e.PublishReobservation(*validated, observation) + } + + if err == nil { + logger.Info("message observed", observation.ZapFields()...) + return nil + } + } else { + if e.suiTxVerifier != nil { + verifiedMsg, verifyErr := e.verify(ctx, observation, *body.ID.TxDigest, logger) + if verifyErr != nil { + err = verifyErr + } else { + observation = &verifiedMsg + } + } + if err == nil { + err = e.PublishMessage(observation) + } + if err == nil { + logger.Info("message observed", observation.ZapFields()...) + } + } if err != nil { suiTransferVerifierFailures.Inc() @@ -351,40 +424,6 @@ func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiR return nil } -func (e *Watcher) verifyAndPublish( - ctx context.Context, - msg *common.MessagePublication, - txDigest string, - logger *zap.Logger, -) error { - if msg == nil { - return errors.New("MessagePublication is nil") - } - - if e.suiTxVerifier != nil { - verifiedMsg, err := e.verify(ctx, msg, txDigest, logger) - - if err != nil { - return err - } - - msg = &verifiedMsg - } - - e.msgChan <- msg //nolint:channelcheck // The channel to the processor is buffered and shared across chains, if it backs up we should stop processing new observations - - suiMessagesConfirmed.Inc() - if msg.IsReobservation { - watchers.ReobservationsByChain.WithLabelValues("sui", "std").Inc() - } - - logger.Info("message observed", - msg.ZapFields()..., - ) - - return nil -} - func (e *Watcher) Run(ctx context.Context) error { p2p.DefaultRegistry.SetNetworkStats(vaa.ChainIDSui, &gossipv1.Heartbeat_Network{ ContractAddress: e.suiMoveEventType, @@ -436,7 +475,7 @@ func (e *Watcher) Run(ctx context.Context) error { if len(dataWithEvents) > 0 { for idx := len(dataWithEvents) - 1; idx >= 0; idx-- { event := dataWithEvents[idx] - err = e.inspectBody(ctx, logger, event.result, false) + err = e.inspectBody(ctx, logger, event.result, nil) if err != nil { logger.Error("inspectBody Error", zap.Error(err)) continue @@ -484,14 +523,13 @@ func (e *Watcher) Run(ctx context.Context) error { logger.Error("sui_fetch_obvs_req context done") return ctx.Err() case r := <-e.obsvReqC: - // node/pkg/node/reobserve.go already enforces the chain id is a valid uint16 - // and only writes to the channel for this chain id. - // If either of the below cases are true, something has gone wrong - if r.ChainId > math.MaxUint16 || vaa.ChainID(r.ChainId) != vaa.ChainIDSui { - panic("invalid chain ID") + validated, err := e.Validate(r) + if err != nil { + watchers.LogInvalidObservationRequest(logger, r, err) + continue } - tx58 := base58.Encode(r.TxHash) + tx58 := base58.Encode(validated.TxHash()) payload := fmt.Sprintf(`{"jsonrpc":"2.0", "id": 1, "method": "sui_getEvents", "params": ["%s"]}`, tx58) @@ -529,7 +567,7 @@ func (e *Watcher) Run(ctx context.Context) error { } for i, chunk := range res.Result { - err := e.inspectBody(ctx, logger, chunk, true) + err := e.inspectBody(ctx, logger, chunk, &validated) if err != nil { logger.Info("sui_fetch_obvs_req skipping event data in result", zap.String("txhash", tx58), zap.Int("index", i), zap.Error(err)) } diff --git a/node/pkg/watchers/sui/watcher_methods_test.go b/node/pkg/watchers/sui/watcher_methods_test.go new file mode 100644 index 00000000000..3614d93d4ff --- /dev/null +++ b/node/pkg/watchers/sui/watcher_methods_test.go @@ -0,0 +1,99 @@ +package sui + +import ( + "testing" + + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +func newTestWatcher(msgC chan<- *common.MessagePublication) *Watcher { + return &Watcher{chainID: vaa.ChainIDSui, msgChan: msgC} +} + +func TestWatcherChainID(t *testing.T) { + tests := []struct { + name string + want vaa.ChainID + }{{name: "returns configured chain", want: vaa.ChainIDSui}} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, newTestWatcher(make(chan *common.MessagePublication, 1)).ChainID()) + }) + } +} + +func TestWatcherValidate(t *testing.T) { + tests := []struct { + name string + req *gossipv1.ObservationRequest + wantErr bool + }{ + {name: "accepts valid request", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}}, + {name: "rejects wrong chain", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}, wantErr: true}, + {name: "accepts empty tx hash", req: &gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui)}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + }) + } +} + +func TestWatcherPublishMessage(t *testing.T) { + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects nil message", wantErr: true}, + {name: "publishes message", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishMessage(tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Same(t, tt.msg, <-msgC) + }) + } +} + +func TestWatcherPublishReobservation(t *testing.T) { + validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}) + require.NoError(t, err) + tests := []struct { + name string + msg *common.MessagePublication + wantErr bool + }{ + {name: "rejects mismatched chain", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum}, wantErr: true}, + {name: "publishes reobservation", msg: &common.MessagePublication{EmitterChain: vaa.ChainIDSui}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + msgC := make(chan *common.MessagePublication, 1) + err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.True(t, tt.msg.IsReobservation) + require.Same(t, tt.msg, <-msgC) + }) + } +} diff --git a/node/pkg/watchers/sui/watcher_test.go b/node/pkg/watchers/sui/watcher_test.go index 6495e4a9fc0..b0ba28ffefc 100644 --- a/node/pkg/watchers/sui/watcher_test.go +++ b/node/pkg/watchers/sui/watcher_test.go @@ -266,7 +266,7 @@ func TestVerifyAndPublish_Samples(t *testing.T) { testCtx := context.TODO() testLogger := zap.NewNop() - // Run verifyAndPublish on samples + // Run verify plus PublishMessage on samples for _, sample := range Samples { t.Run(sample.description, func(t *testing.T) { @@ -285,8 +285,9 @@ func TestVerifyAndPublish_Samples(t *testing.T) { mockApiConnection.SetPastObjects(objectId, version, previousVersion, pastObjects) - // call verify and publish - _ = testWatcher.verifyAndPublish(testCtx, &sample.messagePublication, sample.txDigest, testLogger) + verifiedMsg, err := testWatcher.verify(testCtx, &sample.messagePublication, sample.txDigest, testLogger) + require.NoError(t, err) + require.NoError(t, testWatcher.PublishMessage(&verifiedMsg)) newMessagePublication := <-msgChan require.Equal(t, sample.expectedState, newMessagePublication.VerificationState()) diff --git a/node/pkg/watchers/watcher.go b/node/pkg/watchers/watcher.go new file mode 100644 index 00000000000..c260b83ddda --- /dev/null +++ b/node/pkg/watchers/watcher.go @@ -0,0 +1,99 @@ +package watchers + +import ( + "github.com/certusone/wormhole/node/pkg/common" + gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" + "github.com/wormhole-foundation/wormhole/sdk/vaa" +) + +// ObservationValidator validates a raw gossiped observation request before any +// watcher-specific processing code is allowed to use it. +// +// Implementations should treat Validate as the admission control boundary for +// reobservation requests. The returned ValidObservation should be the canonical +// representation used by downstream watcher logic instead of the raw protobuf +// request. +// +// Validate should perform checks that are fundamental to the safety of turning a +// gossiped observation request into a MessagePublication. Typical checks include: +// +// - the request belongs to this watcher's chain +// - the request's chain ID is a known SDK chain ID +// - the transaction identifier has the watcher-specific shape required by the +// subsequent processing logic +// +// Validate should not publish messages, mutate global state, or perform logging +// as a side effect. It should return a rich error and let the caller decide how +// to report it. +type ObservationValidator interface { + Validate(req *gossipv1.ObservationRequest) (ValidObservation, error) +} + +// MessagePublisher publishes message publications after watcher-specific logic +// has decided that they are ready for the processor. +// +// PublishMessage is the normal publication path for already-built +// MessagePublications. +// +// PublishReobservation is the guarded reobservation publication path. It should +// only be used when a MessagePublication is being emitted in response to a +// previously validated ValidObservation. Implementations should ensure that the +// message remains consistent with the validated observation before delegating to +// PublishMessage. +type MessagePublisher interface { + PublishMessage(msg *common.MessagePublication) error + PublishReobservation(observation ValidObservation, msg *common.MessagePublication) error +} + +// Watcher is the common watcher-side security contract for the +// observation-request to message-publication path. +// +// The purpose of this interface is not to model every behavior of a chain +// watcher. Watchers differ substantially in how they subscribe to chain data, +// fetch transactions, parse logs, and reconstruct message publications. Those +// chain-specific concerns should remain local to each watcher implementation. +// +// Instead, this interface captures the narrow set of behaviors that are shared +// across watchers and are security-sensitive: +// +// - identifying the watcher chain with ChainID +// - validating raw gossiped observation requests with Validate +// - publishing MessagePublications with PublishMessage +// - publishing reobservations through the validated path with +// PublishReobservation +// +// In practical terms, a chain-specific watcher should implement this interface +// on its concrete watcher type and use it to enforce a clean separation between: +// +// - raw protobuf observation requests received from p2p +// - validated observation requests represented as ValidObservation +// - MessagePublications ready to be sent to the processor +// +// Implementers should follow these rules: +// +// - ChainID should return the actual chain handled by this watcher instance, +// not a hard-coded unrelated constant. +// - Validate should be the primary place for watcher-specific request shape +// checks that are required before processing can safely continue. +// - PublishMessage should own the final write to the watcher's publication +// channel. +// - PublishReobservation should only be used for requests that have already +// passed Validate and should delegate into PublishMessage after applying any +// reobservation-specific checks or flags. +// - Watchers should also keep MessagePublication construction in a dedicated +// watcher-local helper or method, for example a BuildMessagePublication +// function. That build step should consume a previously validated +// ValidObservation together with watcher-specific parsed chain data and +// return a MessagePublication ready for publication. Because the parsed +// chain data differs substantially across watchers, this build step is +// intentionally documented here as guidance rather than encoded as a shared +// interface method. +// +// The interface is intentionally small. If a new method does not apply cleanly +// to all watchers without forcing chain-specific parsing or construction details +// into a shared abstraction, it likely does not belong here. +type Watcher interface { + ChainID() vaa.ChainID + ObservationValidator + MessagePublisher +} From 5c1d37cf539f0124abc2a78e96036a0f991fa143 Mon Sep 17 00:00:00 2001 From: John Saigle Date: Tue, 21 Apr 2026 16:03:39 -0400 Subject: [PATCH 2/4] lint --- node/pkg/watchers/solana/client.go | 6 +++--- node/pkg/watchers/solana/client_test.go | 2 +- node/pkg/watchers/solana/reobserve.go | 10 +++++----- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/node/pkg/watchers/solana/client.go b/node/pkg/watchers/solana/client.go index f8c4e63ad4c..841658e9d4b 100644 --- a/node/pkg/watchers/solana/client.go +++ b/node/pkg/watchers/solana/client.go @@ -460,7 +460,7 @@ func (s *SolanaWatcher) Run(ctx context.Context) error { case <-ctx.Done(): return nil case msg := <-s.pumpData: - err := s.processAccountSubscriptionData(ctx, logger, msg, false) + err := s.processAccountSubscriptionData(ctx, logger, msg) if err != nil { p2p.DefaultRegistry.AddErrorCount(s.chainID, 1) solanaConnectionErrors.WithLabelValues(s.networkName, string(s.commitment), "account_subscription_data").Inc() @@ -475,7 +475,7 @@ func (s *SolanaWatcher) Run(ctx context.Context) error { } //nolint:contextcheck // Passed via the 's' object instead of as a parameter. - numObservations, err := s.handleReobservationRequest(validated, s.rpcClient) + numObservations, err := s.handleReobservationRequest(s.ctx, validated, s.rpcClient) if err != nil { logger.Error("failed to process observation request", zap.Uint32("chainID", m.ChainId), @@ -998,7 +998,7 @@ func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc. return s.processMessageAccount(s.logger, data, acc, validated, signature), false } -func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger *zap.Logger, data []byte, isReobservation bool) error { +func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger *zap.Logger, data []byte) error { // Do we have an error on the subscription? var e EventSubscriptionError err := json.Unmarshal(data, &e) diff --git a/node/pkg/watchers/solana/client_test.go b/node/pkg/watchers/solana/client_test.go index c12a7f74952..89642edbdd2 100644 --- a/node/pkg/watchers/solana/client_test.go +++ b/node/pkg/watchers/solana/client_test.go @@ -536,7 +536,7 @@ func TestProcessAccountSubscriptionData(t *testing.T) { s := newTestWatcher(t, vaa.ChainIDSolana, rpc.CommitmentFinalized, msgC) s.rawContract = rawContract - err := s.processAccountSubscriptionData(context.TODO(), zap.NewNop(), tc.data, false) + err := s.processAccountSubscriptionData(context.TODO(), zap.NewNop(), tc.data) if tc.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/solana/reobserve.go b/node/pkg/watchers/solana/reobserve.go index c65b4792a91..23815120321 100644 --- a/node/pkg/watchers/solana/reobserve.go +++ b/node/pkg/watchers/solana/reobserve.go @@ -14,18 +14,18 @@ import ( // handleReobservationRequest performs a reobservation request and publishes any observed transactions. // SECURITY: Only the finalized watcher handles reobservations. Set in configuration of watcher for Solana chains. -func (s *SolanaWatcher) handleReobservationRequest(observation watchers.ValidObservation, rpcClient *rpc.Client) (numObservations uint32, err error) { +func (s *SolanaWatcher) handleReobservationRequest(ctx context.Context, observation watchers.ValidObservation, rpcClient *rpc.Client) (numObservations uint32, err error) { txID := observation.TxHash() if len(txID) == SolanaAccountLen { // Request by account ID acc := solana.PublicKeyFromBytes(txID) s.logger.Info("received observation request with account id", observation.ZapFields(zap.String("account", acc.String()))...) - rCtx, cancel := context.WithTimeout(s.ctx, rpcTimeout) + rCtx, cancel := context.WithTimeout(ctx, rpcTimeout) numObservations, _ = s.fetchMessageAccount(rCtx, rpcClient, acc, 0, &observation, solana.Signature{}) cancel() } else if len(txID) == SolanaSignatureLen { // Request by transaction ID signature := solana.SignatureFromBytes(txID) s.logger.Info("received observation request with transaction id", observation.ZapFields(zap.Stringer("signature", signature))...) - rCtx, cancel := context.WithTimeout(s.ctx, rpcTimeout) + rCtx, cancel := context.WithTimeout(ctx, rpcTimeout) version := uint64(0) result, err := rpcClient.GetTransaction( rCtx, @@ -52,7 +52,7 @@ func (s *SolanaWatcher) handleReobservationRequest(observation watchers.ValidObs if err != nil { return 0, fmt.Errorf("failed to extract transaction for observation request: %v", err) } - numObservations = s.processTransaction(s.ctx, rpcClient, tx, result.Meta, result.Slot, &observation) + numObservations = s.processTransaction(ctx, rpcClient, tx, result.Meta, result.Slot, &observation) } else { return 0, fmt.Errorf("ignoring an observation request of unexpected length: %d", len(txID)) } @@ -71,5 +71,5 @@ func (s *SolanaWatcher) Reobserve(_ context.Context, chainID vaa.ChainID, txID [ if err != nil { return 0, err } - return s.handleReobservationRequest(validated, rpcClient) + return s.handleReobservationRequest(s.ctx, validated, rpcClient) } From e3793514bc29bff025cc00d49c903bfe771832d7 Mon Sep 17 00:00:00 2001 From: John Saigle Date: Tue, 21 Apr 2026 16:22:13 -0400 Subject: [PATCH 3/4] refactor solana to avoid overloading nil --- node/pkg/watchers/solana/client.go | 72 +++++++++++++++---------- node/pkg/watchers/solana/client_test.go | 29 +++++++--- node/pkg/watchers/solana/reobserve.go | 2 +- node/pkg/watchers/solana/tx_for_addr.go | 2 +- 4 files changed, 66 insertions(+), 39 deletions(-) diff --git a/node/pkg/watchers/solana/client.go b/node/pkg/watchers/solana/client.go index 841658e9d4b..42b86186461 100644 --- a/node/pkg/watchers/solana/client.go +++ b/node/pkg/watchers/solana/client.go @@ -684,7 +684,7 @@ func (s *SolanaWatcher) fetchBlock(ctx context.Context, logger *zap.Logger, slot continue } - s.processTransaction(ctx, s.rpcClient, tx, txRpc.Meta, slot, nil) + s.processTransaction(ctx, s.rpcClient, tx, txRpc.Meta, slot, nil, false) } if emptyRetry > 0 && logger.Level().Enabled(zapcore.DebugLevel) { @@ -698,8 +698,7 @@ func (s *SolanaWatcher) fetchBlock(ctx context.Context, logger *zap.Logger, slot } // processTransaction processes a transaction and publishes any Wormhole events. -func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.Client, tx *solana.Transaction, meta *rpc.TransactionMeta, slot uint64, validated *watchers.ValidObservation) (numObservations uint32) { - isReobservation := validated != nil +func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.Client, tx *solana.Transaction, meta *rpc.TransactionMeta, slot uint64, validated *watchers.ValidObservation, isReobservation bool) (numObservations uint32) { // SECURITY: Validate transaction metadata before accessing fields if metadataErr := validateTransactionMeta(meta); metadataErr != nil { if s.logger.Level().Enabled(zapcore.DebugLevel) { @@ -777,7 +776,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C } } } else { - found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, i, validated) + found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, i, validated, isReobservation) if err != nil { s.logger.Error("malformed Wormhole instruction", zap.Error(err), @@ -823,7 +822,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C } } } else { - found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, innerIdx, validated) + found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, innerIdx, validated, isReobservation) if err != nil { s.logger.Error("malformed Wormhole instruction", zap.Error(err), @@ -851,8 +850,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C return } -func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.Client, slot uint64, inst solana.CompiledInstruction, programIndex uint16, tx *solana.Transaction, signature solana.Signature, idx int, validated *watchers.ValidObservation) (bool, error) { - isReobservation := validated != nil +func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.Client, slot uint64, inst solana.CompiledInstruction, programIndex uint16, tx *solana.Transaction, signature solana.Signature, idx int, validated *watchers.ValidObservation, isReobservation bool) (bool, error) { if inst.ProgramIDIndex != programIndex { return false, nil } @@ -995,7 +993,27 @@ func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc. zap.Binary("data", data)) } - return s.processMessageAccount(s.logger, data, acc, validated, signature), false + observation, err := s.processMessageAccount(s.logger, data, acc, signature, validated != nil) + if err != nil { + return 0, false + } + if observation == nil { + return 0, false + } + + if validated != nil { + if err := s.PublishReobservation(*validated, observation); err != nil { + s.logger.Error("failed to publish reobservation", zap.Error(err)) + return 0, false + } + } else { + if err := s.PublishMessage(observation); err != nil { + s.logger.Error("failed to publish message", zap.Error(err)) + return 0, false + } + } + + return 1, false } func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger *zap.Logger, data []byte) error { @@ -1049,7 +1067,16 @@ func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger switch string(data[:3]) { case accountPrefixReliable, accountPrefixUnreliable: acc := solana.PublicKeyFromBytes([]byte(value.Pubkey)) - s.processMessageAccount(logger, data, acc, nil, solana.Signature{}) + observation, err := s.processMessageAccount(logger, data, acc, solana.Signature{}, false) + if err != nil { + return err + } + if observation == nil { + return nil + } + if err := s.PublishMessage(observation); err != nil { + return err + } default: break } @@ -1058,8 +1085,7 @@ func (s *SolanaWatcher) processAccountSubscriptionData(_ context.Context, logger } // SECURITY: Ownership check on account key must be done BEFORE this function is called. -func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, acc solana.PublicKey, validated *watchers.ValidObservation, signature solana.Signature) (numObservations uint32) { - isReobservation := validated != nil +func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, acc solana.PublicKey, signature solana.Signature, isReobservation bool) (*common.MessagePublication, error) { proposal, err := ParseMessagePublicationAccount(data) if err != nil { solanaAccountSkips.WithLabelValues(s.networkName, "parse_transfer_out").Inc() @@ -1068,7 +1094,7 @@ func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, a zap.Stringer("account", acc), zap.Binary("data", data), zap.Error(err)) - return + return nil, err } // SECURITY: defense-in-depth, ensure the consistency level in the account matches the consistency level of the watcher @@ -1078,7 +1104,7 @@ func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, a "failed to parse proposal consistency level", zap.Any("proposal", proposal), zap.Error(err)) - return + return nil, err } if !s.checkCommitment(commitment, isReobservation) { @@ -1089,7 +1115,7 @@ func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, a zap.String("watcher commitment", string(s.commitment)), ) } - return + return nil, nil } // As of 2023-11-09, Pythnet has a bug which is not zeroing out these fields appropriately. This carve out should be removed after a fix is deployed. @@ -1101,7 +1127,7 @@ func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, a "account is not finalized", zap.Stringer("account", acc), zap.Binary("data", data)) - return + return nil, nil } } @@ -1135,26 +1161,14 @@ func (s *SolanaWatcher) processMessageAccount(logger *zap.Logger, data []byte, a // of a shim event where this guardian is not watching the shim contract. Those events should be ignored. if !reliable && len(observation.Payload) == 0 { logger.Debug("ignoring an observation because it is marked unreliable and has a zero length payload, probably from the shim", observation.ZapFields(zap.Stringer("account", acc))...) - return + return nil, nil } if logger.Level().Enabled(s.msgObservedLogLevel) { logger.Log(s.msgObservedLogLevel, "message observed", observation.ZapFields(zap.Stringer("account", acc), zap.Stringer("signature", signature))...) } - if validated != nil { - if err := s.PublishReobservation(*validated, observation); err != nil { - logger.Error("failed to publish reobservation", zap.Error(err)) - return 0 - } - return 1 - } - - if err := s.PublishMessage(observation); err != nil { - logger.Error("failed to publish message", zap.Error(err)) - return 0 - } - return 1 + return observation, nil } // updateLatestBlock() updates the latest block number if the slot passed in is greater than the previous value. diff --git a/node/pkg/watchers/solana/client_test.go b/node/pkg/watchers/solana/client_test.go index 89642edbdd2..2f114fce31f 100644 --- a/node/pkg/watchers/solana/client_test.go +++ b/node/pkg/watchers/solana/client_test.go @@ -15,7 +15,6 @@ import ( "github.com/certusone/wormhole/node/pkg/common" gossipv1 "github.com/certusone/wormhole/node/pkg/proto/gossip/v1" - "github.com/certusone/wormhole/node/pkg/watchers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/wormhole-foundation/wormhole/sdk/vaa" @@ -302,6 +301,7 @@ func TestProcessMessageAccount(t *testing.T) { wantCount uint32 wantUnreliable bool wantReobs bool + wantErr bool }{ { name: "publishes_reliable", @@ -360,6 +360,7 @@ func TestProcessMessageAccount(t *testing.T) { prefix: accountPrefixReliable, payload: []byte("hello"), consistencyLevel: 99, + wantErr: true, }, { name: "skips_unreliable_empty_payload", @@ -414,14 +415,26 @@ func TestProcessMessageAccount(t *testing.T) { data := encodeMessagePublicationAccount(t, tc.prefix, proposal) acc := solana.PublicKeyFromBytes(bytes.Repeat([]byte{0x11}, solana.PublicKeyLength)) - var validated *watchers.ValidObservation + observation, err := s.processMessageAccount(s.logger, data, acc, solana.Signature{}, tc.isReobservation) + if tc.wantCount == 0 { + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + assert.Nil(t, observation) + return + } + require.NoError(t, err) + require.NotNil(t, observation) if tc.isReobservation { obs, err := s.Validate(&gossipv1.ObservationRequest{ChainId: uint32(tc.chainID), TxHash: acc.Bytes()}) require.NoError(t, err) - validated = &obs + require.NoError(t, s.PublishReobservation(obs, observation)) + } else { + require.NoError(t, s.PublishMessage(observation)) } - num := s.processMessageAccount(s.logger, data, acc, validated, solana.Signature{}) - assert.Equal(t, tc.wantCount, num) + assert.Equal(t, tc.wantCount, uint32(len(msgC))) if tc.wantCount == 0 { assert.Equal(t, 0, len(msgC)) @@ -607,7 +620,7 @@ func TestProcessInstructionEarlyReturns(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - found, err := s.processInstruction(context.TODO(), nil, 1, tc.inst, 0, tx, signature, 0, nil) + found, err := s.processInstruction(context.TODO(), nil, 1, tc.inst, 0, tx, signature, 0, nil, false) if tc.wantErr { require.Error(t, err) return @@ -671,7 +684,7 @@ func TestProcessInstructionValidPostMessage(t *testing.T) { Accounts: []uint16{0, 1, 0, 0, 0, 0, 0, 0}, } - found, err := s.processInstruction(context.Background(), rpcClient, 1, inst, 0, tx, tx.Signatures[0], 0, nil) + found, err := s.processInstruction(context.Background(), rpcClient, 1, inst, 0, tx, tx.Signatures[0], 0, nil, false) require.NoError(t, err) assert.True(t, found) @@ -884,7 +897,7 @@ func TestProcessTransaction(t *testing.T) { InnerInstructions: tc.innerInstructions, } - num := s.processTransaction(context.Background(), rpcClient, tx, meta, 42, nil) + num := s.processTransaction(context.Background(), rpcClient, tx, meta, 42, nil, false) assert.Equal(t, tc.wantObservations, num) // Drain published messages and verify count. diff --git a/node/pkg/watchers/solana/reobserve.go b/node/pkg/watchers/solana/reobserve.go index 23815120321..ad8dcaf79da 100644 --- a/node/pkg/watchers/solana/reobserve.go +++ b/node/pkg/watchers/solana/reobserve.go @@ -52,7 +52,7 @@ func (s *SolanaWatcher) handleReobservationRequest(ctx context.Context, observat if err != nil { return 0, fmt.Errorf("failed to extract transaction for observation request: %v", err) } - numObservations = s.processTransaction(ctx, rpcClient, tx, result.Meta, result.Slot, &observation) + numObservations = s.processTransaction(ctx, rpcClient, tx, result.Meta, result.Slot, &observation, true) } else { return 0, fmt.Errorf("ignoring an observation request of unexpected length: %d", len(txID)) } diff --git a/node/pkg/watchers/solana/tx_for_addr.go b/node/pkg/watchers/solana/tx_for_addr.go index 5a718710d12..0536fd160ca 100644 --- a/node/pkg/watchers/solana/tx_for_addr.go +++ b/node/pkg/watchers/solana/tx_for_addr.go @@ -193,7 +193,7 @@ func (s *SolanaWatcher) processTransactionWithRetry(signature solana.Signature) return } - _ = s.processTransaction(s.ctx, s.rpcClient, tx, result.Meta, result.Slot, nil) + _ = s.processTransaction(s.ctx, s.rpcClient, tx, result.Meta, result.Slot, nil, false) return } From 827609f7c733ff2b1a8aef940292c123c4e8e412 Mon Sep 17 00:00:00 2001 From: John Saigle Date: Wed, 22 Apr 2026 08:51:01 -0400 Subject: [PATCH 4/4] avoid err shadowing; make var names clearer --- node/pkg/watchers/algorand/watcher.go | 24 +++++------ .../watchers/algorand/watcher_methods_test.go | 8 ++-- node/pkg/watchers/algorand/watcher_test.go | 4 +- node/pkg/watchers/aptos/watcher.go | 22 +++++----- .../watchers/aptos/watcher_methods_test.go | 8 ++-- node/pkg/watchers/aptos/watcher_test.go | 4 +- node/pkg/watchers/cosmwasm/watcher.go | 12 +++--- .../watchers/cosmwasm/watcher_methods_test.go | 8 ++-- node/pkg/watchers/evm/reobserve.go | 4 +- node/pkg/watchers/evm/watcher.go | 8 ++-- node/pkg/watchers/evm/watcher_methods_test.go | 8 ++-- node/pkg/watchers/ibc/watcher.go | 18 ++++---- node/pkg/watchers/ibc/watcher_methods_test.go | 8 ++-- node/pkg/watchers/mock/watcher.go | 8 ++-- .../pkg/watchers/mock/watcher_methods_test.go | 8 ++-- node/pkg/watchers/near/watcher.go | 12 +++--- .../pkg/watchers/near/watcher_methods_test.go | 8 ++-- node/pkg/watchers/observation_test.go | 30 +++++++------- node/pkg/watchers/solana/client.go | 34 +++++++-------- node/pkg/watchers/solana/reobserve.go | 4 +- .../watchers/solana/watcher_methods_test.go | 8 ++-- node/pkg/watchers/sui/watcher.go | 41 ++++++++++--------- node/pkg/watchers/sui/watcher_methods_test.go | 8 ++-- 23 files changed, 149 insertions(+), 148 deletions(-) diff --git a/node/pkg/watchers/algorand/watcher.go b/node/pkg/watchers/algorand/watcher.go index 3774e128de1..5e5ea1346f4 100644 --- a/node/pkg/watchers/algorand/watcher.go +++ b/node/pkg/watchers/algorand/watcher.go @@ -95,12 +95,12 @@ func NewWatcher( } func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, e.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, e.chainID) if err != nil { return watchers.ValidObservation{}, err } - return validated, nil + return validatedObservation, nil } func (e *Watcher) ChainID() vaa.ChainID { @@ -193,8 +193,8 @@ func gatherObservations(e *Watcher, t types.SignedTxnWithAD, depth int, logger * // lookAtTxn takes an outer transaction from the block.payset and gathers // observations from messages emitted in nested inner transactions // then passes them on the relevant channels -func lookAtTxn(e *Watcher, t types.SignedTxnInBlock, b types.Block, logger *zap.Logger, validated *watchers.ValidObservation) { - isReobservation := validated != nil +func lookAtTxn(e *Watcher, t types.SignedTxnInBlock, b types.Block, logger *zap.Logger, validatedObservation *watchers.ValidObservation) { + isReobservation := validatedObservation != nil observations := gatherObservations(e, t.SignedTxnWithAD, 0, logger) @@ -234,8 +234,8 @@ func lookAtTxn(e *Watcher, t types.SignedTxnInBlock, b types.Block, logger *zap. logger.Info("message observed", observation.ZapFields()...) - if validated != nil { - if err := e.PublishReobservation(*validated, observation); err != nil { + if validatedObservation != nil { + if err := e.PublishReobservation(*validatedObservation, observation); err != nil { logger.Error("failed to publish reobservation", zap.Error(err)) continue } @@ -304,7 +304,7 @@ func (e *Watcher) Run(ctx context.Context) error { case <-ctx.Done(): return nil case r := <-e.obsvReqC: - validated, err := e.Validate(r) + validatedObservation, err := e.Validate(r) if err != nil { watchers.LogInvalidObservationRequest(logger, r, err) p2p.DefaultRegistry.AddErrorCount(vaa.ChainIDAlgorand, 1) @@ -313,13 +313,13 @@ func (e *Watcher) Run(ctx context.Context) error { logger.Info( "received observation request", - validated.ZapFields( - zap.String("tx_hash", hex.EncodeToString(validated.TxHash())), - zap.String("base32_tx_hash", base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(validated.TxHash())), + validatedObservation.ZapFields( + zap.String("tx_hash", hex.EncodeToString(validatedObservation.TxHash())), + zap.String("base32_tx_hash", base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(validatedObservation.TxHash())), )..., ) - result, err := indexerClient.SearchForTransactions().TXID(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(validated.TxHash())).Do(ctx) + result, err := indexerClient.SearchForTransactions().TXID(base32.StdEncoding.WithPadding(base32.NoPadding).EncodeToString(validatedObservation.TxHash())).Do(ctx) if err != nil { logger.Error("SearchForTransactions", zap.Error(err)) p2p.DefaultRegistry.AddErrorCount(vaa.ChainIDAlgorand, 1) @@ -336,7 +336,7 @@ func (e *Watcher) Run(ctx context.Context) error { } for _, element := range block.Payset { - lookAtTxn(e, element, block, logger, &validated) + lookAtTxn(e, element, block, logger, &validatedObservation) } } diff --git a/node/pkg/watchers/algorand/watcher_methods_test.go b/node/pkg/watchers/algorand/watcher_methods_test.go index d0f0ca65e1e..698913995fc 100644 --- a/node/pkg/watchers/algorand/watcher_methods_test.go +++ b/node/pkg/watchers/algorand/watcher_methods_test.go @@ -44,13 +44,13 @@ func TestWatcherValidate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { w := newMethodTestWatcher(make(chan *common.MessagePublication, 1)) - validated, err := w.Validate(tt.req) + validatedObservation, err := w.Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -81,7 +81,7 @@ func TestWatcherPublishMessage(t *testing.T) { } func TestWatcherPublishReobservation(t *testing.T) { - validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand), TxHash: make([]byte, common.TxIDLenMin)}) + validatedObservation, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand), TxHash: make([]byte, common.TxIDLenMin)}) require.NoError(t, err) tests := []struct { @@ -97,7 +97,7 @@ func TestWatcherPublishReobservation(t *testing.T) { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) w := newMethodTestWatcher(msgC) - err := w.PublishReobservation(validated, tt.msg) + err := w.PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/algorand/watcher_test.go b/node/pkg/watchers/algorand/watcher_test.go index 075c6a6d312..ba448b717df 100644 --- a/node/pkg/watchers/algorand/watcher_test.go +++ b/node/pkg/watchers/algorand/watcher_test.go @@ -331,10 +331,10 @@ func TestLookAtTxn_Reobservation(t *testing.T) { BlockHeader: types.BlockHeader{TimeStamp: 1700000000}, } - validated, err := watchers.ValidateObservationRequest(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand), TxHash: make([]byte, common.TxIDLenMin)}, vaa.ChainIDAlgorand) + validatedObservation, err := watchers.ValidateObservationRequest(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAlgorand), TxHash: make([]byte, common.TxIDLenMin)}, vaa.ChainIDAlgorand) require.NoError(t, err) - lookAtTxn(w, txn, block, logger, &validated) + lookAtTxn(w, txn, block, logger, &validatedObservation) select { case msg := <-msgC: diff --git a/node/pkg/watchers/aptos/watcher.go b/node/pkg/watchers/aptos/watcher.go index dc0cddcf8f3..46f6ba80925 100644 --- a/node/pkg/watchers/aptos/watcher.go +++ b/node/pkg/watchers/aptos/watcher.go @@ -79,7 +79,7 @@ func NewWatcher( } func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, e.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, e.chainID) if err != nil { return watchers.ValidObservation{}, err } @@ -92,11 +92,11 @@ func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObse // SECURITY: This acts as a bounds check for the BigEndian.Uint64 call in the // reobservation handler. const aptosTxIDExpectedLen = 32 - if len(validated.TxHash()) < aptosTxIDExpectedLen { + if len(validatedObservation.TxHash()) < aptosTxIDExpectedLen { return watchers.ValidObservation{}, fmt.Errorf("invalid TxID: too short") } - return validated, nil + return validatedObservation, nil } func (e *Watcher) ChainID() vaa.ChainID { @@ -170,19 +170,19 @@ func (e *Watcher) Run(ctx context.Context) error { case <-ctx.Done(): return ctx.Err() case r := <-e.obsvReqC: - validated, err := e.Validate(r) + validatedObservation, err := e.Validate(r) if err != nil { watchers.LogInvalidObservationRequest(logger, r, err) p2p.DefaultRegistry.AddErrorCount(e.chainID, 1) continue } - txHash := validated.TxHash() + txHash := validatedObservation.TxHash() // uint64 will read the *first* 8 bytes, but the sequence is stored in the *last* 8. nativeSeq := binary.BigEndian.Uint64(txHash[24:]) - logger.Info("received observation request", validated.ZapFields(zap.Uint64("tx_hash", nativeSeq))...) + logger.Info("received observation request", validatedObservation.ZapFields(zap.Uint64("tx_hash", nativeSeq))...) s := fmt.Sprintf(`%s?start=%d&limit=1`, eventsEndpoint, nativeSeq) @@ -218,7 +218,7 @@ func (e *Watcher) Run(ctx context.Context) error { if !data.Exists() { break } - e.observeData(logger, data, nativeSeq, &validated) + e.observeData(logger, data, nativeSeq, &validatedObservation) } case <-timer.C: @@ -338,8 +338,8 @@ func (e *Watcher) retrievePayload(s string) ([]byte, error) { return body, err } -func (w *Watcher) observeData(logger *zap.Logger, data gjson.Result, nativeSeq uint64, validated *watchers.ValidObservation) { - isReobservation := validated != nil +func (w *Watcher) observeData(logger *zap.Logger, data gjson.Result, nativeSeq uint64, validatedObservation *watchers.ValidObservation) { + isReobservation := validatedObservation != nil em := data.Get("sender") if !em.Exists() { logger.Error("sender field missing") @@ -424,8 +424,8 @@ func (w *Watcher) observeData(logger *zap.Logger, data gjson.Result, nativeSeq u logger.Info("message observed", observation.ZapFields(zap.String("txHash", observation.TxIDString()), zap.Uint8("consistencyLevel", observation.ConsistencyLevel))...) - if validated != nil { - if err := w.PublishReobservation(*validated, observation); err != nil { + if validatedObservation != nil { + if err := w.PublishReobservation(*validatedObservation, observation); err != nil { logger.Error("failed to publish reobservation", zap.Error(err)) return } diff --git a/node/pkg/watchers/aptos/watcher_methods_test.go b/node/pkg/watchers/aptos/watcher_methods_test.go index c70f8b40672..3ebb39ff033 100644 --- a/node/pkg/watchers/aptos/watcher_methods_test.go +++ b/node/pkg/watchers/aptos/watcher_methods_test.go @@ -40,13 +40,13 @@ func TestWatcherValidate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -76,7 +76,7 @@ func TestWatcherPublishMessage(t *testing.T) { } func TestWatcherPublishReobservation(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, 32)}) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, 32)}) require.NoError(t, err) tests := []struct { @@ -91,7 +91,7 @@ func TestWatcherPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + err := newTestWatcher(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/aptos/watcher_test.go b/node/pkg/watchers/aptos/watcher_test.go index d2245e14b60..5db175dcd79 100644 --- a/node/pkg/watchers/aptos/watcher_test.go +++ b/node/pkg/watchers/aptos/watcher_test.go @@ -201,10 +201,10 @@ func TestObserveDataFields(t *testing.T) { "consistency_level": "15" }` - validated, err := watchers.ValidateObservationRequest(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, common.TxIDLenMin)}, vaa.ChainIDAptos) + validatedObservation, err := watchers.ValidateObservationRequest(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDAptos), TxHash: make([]byte, common.TxIDLenMin)}, vaa.ChainIDAptos) require.NoError(t, err) - w.observeData(logger, gjson.Parse(json), 123, &validated) + w.observeData(logger, gjson.Parse(json), 123, &validatedObservation) require.Len(t, msgC, 1) msg := <-msgC diff --git a/node/pkg/watchers/cosmwasm/watcher.go b/node/pkg/watchers/cosmwasm/watcher.go index 930f0b0ef2b..92f51575bdc 100644 --- a/node/pkg/watchers/cosmwasm/watcher.go +++ b/node/pkg/watchers/cosmwasm/watcher.go @@ -146,12 +146,12 @@ func NewWatcher( } func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, e.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, e.chainID) if err != nil { return watchers.ValidObservation{}, err } - return validated, nil + return validatedObservation, nil } func (e *Watcher) ChainID() vaa.ChainID { @@ -288,7 +288,7 @@ func (e *Watcher) Run(ctx context.Context) error { case <-ctx.Done(): return nil case r := <-e.obsvReqC: - validated, err := e.Validate(r) + validatedObservation, err := e.Validate(r) if err != nil { watchers.LogInvalidObservationRequest(logger, r, err) continue @@ -297,9 +297,9 @@ func (e *Watcher) Run(ctx context.Context) error { // SECURITY: Directly using data for URL path is scary. // Potential for directory traversal attacks to return the incorrect data // This is hex encoded so it's acceptable but be careful changing this logic. - tx := hex.EncodeToString(validated.TxHash()) + tx := hex.EncodeToString(validatedObservation.TxHash()) - logger.Info("received observation request", validated.ZapFields(zap.String("network", e.networkName), zap.String("tx_hash", tx))...) + logger.Info("received observation request", validatedObservation.ZapFields(zap.String("network", e.networkName), zap.String("tx_hash", tx))...) client := &http.Client{ Timeout: time.Second * 5, @@ -351,7 +351,7 @@ func (e *Watcher) Run(ctx context.Context) error { msgs := EventsToMessagePublications(e.contract, txHash, events.Array(), logger, e.chainID, contractAddressLogKey, e.b64Encoded) for _, msg := range msgs { - if err := e.PublishReobservation(validated, msg); err != nil { + if err := e.PublishReobservation(validatedObservation, msg); err != nil { logger.Error("failed to publish reobservation", zap.Error(err)) continue } diff --git a/node/pkg/watchers/cosmwasm/watcher_methods_test.go b/node/pkg/watchers/cosmwasm/watcher_methods_test.go index b30503529b2..310c7b52647 100644 --- a/node/pkg/watchers/cosmwasm/watcher_methods_test.go +++ b/node/pkg/watchers/cosmwasm/watcher_methods_test.go @@ -40,13 +40,13 @@ func TestWatcherValidate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -76,7 +76,7 @@ func TestWatcherPublishMessage(t *testing.T) { } func TestWatcherPublishReobservation(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDTerra), TxHash: make([]byte, common.TxIDLenMin)}) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDTerra), TxHash: make([]byte, common.TxIDLenMin)}) require.NoError(t, err) tests := []struct { @@ -91,7 +91,7 @@ func TestWatcherPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + err := newTestWatcher(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/evm/reobserve.go b/node/pkg/watchers/evm/reobserve.go index 1445d573eb6..09c8c4f0852 100644 --- a/node/pkg/watchers/evm/reobserve.go +++ b/node/pkg/watchers/evm/reobserve.go @@ -173,9 +173,9 @@ func (w *Watcher) Reobserve(ctx context.Context, chainID vaa.ChainID, txID []byt } // Finally, do the reobservation and return the number of messages observed. - validated, err := w.Validate(&gossipv1.ObservationRequest{ChainId: uint32(chainID), TxHash: txID}) + validatedObservation, err := w.Validate(&gossipv1.ObservationRequest{ChainId: uint32(chainID), TxHash: txID}) if err != nil { return 0, err } - return w.handleReobservationRequest(ctx, validated, ethConn, finalized, safe) + return w.handleReobservationRequest(ctx, validatedObservation, ethConn, finalized, safe) } diff --git a/node/pkg/watchers/evm/watcher.go b/node/pkg/watchers/evm/watcher.go index 97c8fd4aa55..942ba2cfd78 100644 --- a/node/pkg/watchers/evm/watcher.go +++ b/node/pkg/watchers/evm/watcher.go @@ -232,12 +232,12 @@ func NewEthWatcher( } func (w *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, w.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, w.chainID) if err != nil { return watchers.ValidObservation{}, err } - return validated, nil + return validatedObservation, nil } func (w *Watcher) ChainID() vaa.ChainID { @@ -464,14 +464,14 @@ func (w *Watcher) Run(parentCtx context.Context) error { case <-ctx.Done(): return nil case r := <-w.obsvReqC: - validated, err := w.Validate(r) + validatedObservation, err := w.Validate(r) if err != nil { watchers.LogInvalidObservationRequest(logger, r, err) continue } numObservations, err := w.handleReobservationRequest( ctx, - validated, + validatedObservation, w.ethConn, atomic.LoadUint64(&w.latestFinalizedBlockNumber), atomic.LoadUint64(&w.latestSafeBlockNumber), diff --git a/node/pkg/watchers/evm/watcher_methods_test.go b/node/pkg/watchers/evm/watcher_methods_test.go index cd5842a13dd..859bedd68ff 100644 --- a/node/pkg/watchers/evm/watcher_methods_test.go +++ b/node/pkg/watchers/evm/watcher_methods_test.go @@ -39,13 +39,13 @@ func TestWatcherMethodValidate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -74,7 +74,7 @@ func TestWatcherMethodPublishMessage(t *testing.T) { } func TestWatcherMethodPublishReobservation(t *testing.T) { - validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}) + validatedObservation, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDEthereum), TxHash: make([]byte, common.TxIDLenMin)}) require.NoError(t, err) tests := []struct { name string @@ -87,7 +87,7 @@ func TestWatcherMethodPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newMethodTestWatcher(msgC).PublishReobservation(validated, tt.msg) + err := newMethodTestWatcher(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/ibc/watcher.go b/node/pkg/watchers/ibc/watcher.go index d5df57c128a..2cf7cc192b0 100644 --- a/node/pkg/watchers/ibc/watcher.go +++ b/node/pkg/watchers/ibc/watcher.go @@ -181,12 +181,12 @@ func NewWatcher( } func (ce *chainEntry) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, ce.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, ce.chainID) if err != nil { return watchers.ValidObservation{}, err } - return validated, nil + return validatedObservation, nil } func (ce *chainEntry) ChainID() vaa.ChainID { @@ -473,7 +473,7 @@ func (w *Watcher) handleObservationRequests(ctx context.Context, ce *chainEntry) case <-ctx.Done(): return nil case r := <-ce.obsvReqC: - validated, err := ce.Validate(r) + validatedObservation, err := ce.Validate(r) if err != nil { watchers.LogInvalidObservationRequest(w.logger, r, err, zap.String("chain", ce.chainName)) continue @@ -482,8 +482,8 @@ func (w *Watcher) handleObservationRequests(ctx context.Context, ce *chainEntry) // SECURITY: Directly using data for URL path is scary. // Potential for directory traversal attacks to return the incorrect data // This is hex encoded so it's acceptable but be careful changing this logic. - reqTxHashStr := hex.EncodeToString(validated.TxHash()) - w.logger.Info("received observation request", validated.ZapFields(zap.String("chain", ce.chainName), zap.String("txHash", reqTxHashStr))...) + reqTxHashStr := hex.EncodeToString(validatedObservation.TxHash()) + w.logger.Info("received observation request", validatedObservation.ZapFields(zap.String("chain", ce.chainName), zap.String("txHash", reqTxHashStr))...) client := &http.Client{ Timeout: time.Second * 5, @@ -539,7 +539,7 @@ func (w *Watcher) handleObservationRequests(ctx context.Context, ce *chainEntry) } if evt != nil { - if err := w.processIbcReceivePublishEvent(evt, "reobservation", &validated); err != nil { + if err := w.processIbcReceivePublishEvent(evt, "reobservation", &validatedObservation); err != nil { return fmt.Errorf("failed to process reobserved IBC event: %w", err) } } @@ -637,7 +637,7 @@ func parseIbcReceivePublishEvent(logger *zap.Logger, desiredContract string, eve } // processIbcReceivePublishEvent takes an IBC event, maps it to a message publication and publishes it. -func (w *Watcher) processIbcReceivePublishEvent(evt *ibcReceivePublishEvent, observationType string, validated *watchers.ValidObservation) error { +func (w *Watcher) processIbcReceivePublishEvent(evt *ibcReceivePublishEvent, observationType string, validatedObservation *watchers.ValidObservation) error { // SECURITY: The ibc watcher is the only watcher that can handle multiple chain IDs // To make this safe, it has a mapping from channel ID to chain IDs that it uses @@ -725,8 +725,8 @@ func (w *Watcher) processIbcReceivePublishEvent(evt *ibcReceivePublishEvent, obs zap.Uint8("ConsistencyLevel", evt.Msg.ConsistencyLevel), ) - if validated != nil { - if err := ce.PublishReobservation(*validated, evt.Msg); err != nil { + if validatedObservation != nil { + if err := ce.PublishReobservation(*validatedObservation, evt.Msg); err != nil { return err } } else { diff --git a/node/pkg/watchers/ibc/watcher_methods_test.go b/node/pkg/watchers/ibc/watcher_methods_test.go index b855b029408..53a1fa26802 100644 --- a/node/pkg/watchers/ibc/watcher_methods_test.go +++ b/node/pkg/watchers/ibc/watcher_methods_test.go @@ -40,13 +40,13 @@ func TestChainEntryValidate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newTestChainEntry(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newTestChainEntry(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -76,7 +76,7 @@ func TestChainEntryPublishMessage(t *testing.T) { } func TestChainEntryPublishReobservation(t *testing.T) { - validated, err := newTestChainEntry(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDOsmosis), TxHash: make([]byte, common.TxIDLenMin)}) + validatedObservation, err := newTestChainEntry(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDOsmosis), TxHash: make([]byte, common.TxIDLenMin)}) require.NoError(t, err) tests := []struct { @@ -91,7 +91,7 @@ func TestChainEntryPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newTestChainEntry(msgC).PublishReobservation(validated, tt.msg) + err := newTestChainEntry(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/mock/watcher.go b/node/pkg/watchers/mock/watcher.go index e612c038e47..168bb9f3896 100644 --- a/node/pkg/watchers/mock/watcher.go +++ b/node/pkg/watchers/mock/watcher.go @@ -68,17 +68,17 @@ func (w *Watcher) Run(ctx context.Context) error { case gs := <-w.config.MockSetC: w.setC <- gs //nolint:channelcheck // Will only block this mock watcher case o := <-w.obsvReqC: - validated, err := w.Validate(o) + validatedObservation, err := w.Validate(o) if err != nil { watchers.LogInvalidObservationRequest(logger, o, err) continue } - hash := eth_common.BytesToHash(validated.TxHash()) - logger.Info("received observation request", validated.ZapFields(zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", hash.Hex()))...) + hash := eth_common.BytesToHash(validatedObservation.TxHash()) + logger.Info("received observation request", validatedObservation.ZapFields(zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", hash.Hex()))...) msg, ok := w.config.ObservationDb[hash] if ok { msg2 := *msg - if err := w.PublishReobservation(validated, &msg2); err != nil { + if err := w.PublishReobservation(validatedObservation, &msg2); err != nil { logger.Error("failed to publish reobservation", zap.Error(err)) } } diff --git a/node/pkg/watchers/mock/watcher_methods_test.go b/node/pkg/watchers/mock/watcher_methods_test.go index 3b9bcb68607..2c457831564 100644 --- a/node/pkg/watchers/mock/watcher_methods_test.go +++ b/node/pkg/watchers/mock/watcher_methods_test.go @@ -37,13 +37,13 @@ func TestWatcherValidate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -72,7 +72,7 @@ func TestWatcherPublishMessage(t *testing.T) { } func TestWatcherPublishReobservation(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}) require.NoError(t, err) tests := []struct { name string @@ -85,7 +85,7 @@ func TestWatcherPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + err := newTestWatcher(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/near/watcher.go b/node/pkg/watchers/near/watcher.go index 1030db2e0f8..84f7dd58c18 100644 --- a/node/pkg/watchers/near/watcher.go +++ b/node/pkg/watchers/near/watcher.go @@ -133,12 +133,12 @@ func newTransactionProcessingJob(txHash string, senderAccountId string, isReobse } func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, e.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, e.chainID) if err != nil { return watchers.ValidObservation{}, err } - return validated, nil + return validatedObservation, nil } func (e *Watcher) ChainID() vaa.ChainID { @@ -242,21 +242,21 @@ func (e *Watcher) runObsvReqProcessor(ctx context.Context) error { case <-ctx.Done(): return ctx.Err() case r := <-e.obsvReqC: - validated, err := e.Validate(r) + validatedObservation, err := e.Validate(r) if err != nil { watchers.LogInvalidObservationRequest(logger, r, err, zap.String("txIDBase58", base58.Encode(r.GetTxHash()))) continue } - txHash := base58.Encode(validated.TxHash()) + txHash := base58.Encode(validatedObservation.TxHash()) - logger.Info("received observation request", validated.ZapFields(zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", txHash))...) + logger.Info("received observation request", validatedObservation.ZapFields(zap.String("log_msg_type", "obsv_req_received"), zap.String("tx_hash", txHash))...) // TODO e.wormholeContract is not the correct value for senderAccountId. Instead, it should be the account id of the transaction sender. // This value is used by NEAR to determine which shard to query. An incorrect value here is not a security risk but could lead to reobservation requests failing. // Guardians currently run nodes for all shards and the API seems to be returning the correct results independent of the set senderAccountId but this could change in the future. // Fixing this would require adding the transaction sender account ID to the observation request. - job := newTransactionProcessingJob(txHash, e.wormholeAccount, true, &validated) + job := newTransactionProcessingJob(txHash, e.wormholeAccount, true, &validatedObservation) err = e.schedule(ctx, job, time.Nanosecond) if err != nil { // Error-level logging here because this is after an re-observation request already, which should be infrequent diff --git a/node/pkg/watchers/near/watcher_methods_test.go b/node/pkg/watchers/near/watcher_methods_test.go index 192a3d29330..c110228ad75 100644 --- a/node/pkg/watchers/near/watcher_methods_test.go +++ b/node/pkg/watchers/near/watcher_methods_test.go @@ -38,13 +38,13 @@ func TestWatcherValidate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -73,7 +73,7 @@ func TestWatcherPublishMessage(t *testing.T) { } func TestWatcherPublishReobservation(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDNear), TxHash: make([]byte, common.TxIDLenMin)}) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDNear), TxHash: make([]byte, common.TxIDLenMin)}) require.NoError(t, err) tests := []struct { name string @@ -86,7 +86,7 @@ func TestWatcherPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + err := newTestWatcher(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/observation_test.go b/node/pkg/watchers/observation_test.go index 29fa8e2d886..03f939e14db 100644 --- a/node/pkg/watchers/observation_test.go +++ b/node/pkg/watchers/observation_test.go @@ -20,19 +20,19 @@ func TestValidateObservationRequest(t *testing.T) { }) t.Run("accepts expected chain", func(t *testing.T) { - validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + validatedObservation, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ ChainId: uint32(vaa.ChainIDSui), TxHash: []byte{1, 2, 3}, Timestamp: 1234, }, vaa.ChainIDSui) require.NoError(t, err) - assert.Equal(t, vaa.ChainIDSui, validated.ChainID()) - assert.Equal(t, []byte{1, 2, 3}, validated.TxHash()) - assert.Equal(t, int64(1234), validated.Timestamp()) + assert.Equal(t, vaa.ChainIDSui, validatedObservation.ChainID()) + assert.Equal(t, []byte{1, 2, 3}, validatedObservation.TxHash()) + assert.Equal(t, int64(1234), validatedObservation.Timestamp()) - original := validated.TxHash() + original := validatedObservation.TxHash() original[0] = 99 - assert.Equal(t, []byte{1, 2, 3}, validated.TxHash()) + assert.Equal(t, []byte{1, 2, 3}, validatedObservation.TxHash()) }) t.Run("rejects unknown chain number", func(t *testing.T) { @@ -54,45 +54,45 @@ func TestValidateObservationRequest(t *testing.T) { }) t.Run("tx hash length helper", func(t *testing.T) { - validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + validatedObservation, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin), }, vaa.ChainIDSui) require.NoError(t, err) - require.NoError(t, validated.RequireTxHashLength(common.TxIDLenMin)) - require.Error(t, validated.RequireTxHashLength(64)) + require.NoError(t, validatedObservation.RequireTxHashLength(common.TxIDLenMin)) + require.Error(t, validatedObservation.RequireTxHashLength(64)) }) } func TestValidateReobservedMessage(t *testing.T) { - validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + validatedObservation, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin), }, vaa.ChainIDSui) require.NoError(t, err) t.Run("rejects nil message", func(t *testing.T) { - require.Error(t, ValidateReobservedMessage(validated, nil)) + require.Error(t, ValidateReobservedMessage(validatedObservation, nil)) }) t.Run("rejects mismatched chain", func(t *testing.T) { msg := &common.MessagePublication{EmitterChain: vaa.ChainIDEthereum} - require.Error(t, ValidateReobservedMessage(validated, msg)) + require.Error(t, ValidateReobservedMessage(validatedObservation, msg)) }) msg := &common.MessagePublication{EmitterChain: vaa.ChainIDSui} - require.NoError(t, ValidateReobservedMessage(validated, msg)) + require.NoError(t, ValidateReobservedMessage(validatedObservation, msg)) } func TestValidObservationZapFields(t *testing.T) { - validated, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ + validatedObservation, err := ValidateObservationRequest(&gossipv1.ObservationRequest{ ChainId: uint32(vaa.ChainIDSui), TxHash: []byte{0xAA, 0xBB}, Timestamp: 42, }, vaa.ChainIDSui) require.NoError(t, err) - fields := validated.ZapFields(zap.String("extra", "value")) + fields := validatedObservation.ZapFields(zap.String("extra", "value")) require.Len(t, fields, 5) assert.Equal(t, zap.String("extra", "value").Key, fields[0].Key) assert.Equal(t, "chainID", fields[1].Key) diff --git a/node/pkg/watchers/solana/client.go b/node/pkg/watchers/solana/client.go index 42b86186461..2d8109c70a1 100644 --- a/node/pkg/watchers/solana/client.go +++ b/node/pkg/watchers/solana/client.go @@ -286,18 +286,18 @@ func NewSolanaWatcher( } func (s *SolanaWatcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, s.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, s.chainID) if err != nil { return watchers.ValidObservation{}, err } // Solana reobservations accept either a message account public key or a // transaction signature, so both wire lengths are valid here. - if err := validated.RequireTxHashLength(SolanaAccountLen, SolanaSignatureLen); err != nil { + if err := validatedObservation.RequireTxHashLength(SolanaAccountLen, SolanaSignatureLen); err != nil { return watchers.ValidObservation{}, err } - return validated, nil + return validatedObservation, nil } func (s *SolanaWatcher) ChainID() vaa.ChainID { @@ -468,14 +468,14 @@ func (s *SolanaWatcher) Run(ctx context.Context) error { return err } case m := <-s.obsvReqC: - validated, err := s.Validate(m) + validatedObservation, err := s.Validate(m) if err != nil { watchers.LogInvalidObservationRequest(logger, m, err) continue } //nolint:contextcheck // Passed via the 's' object instead of as a parameter. - numObservations, err := s.handleReobservationRequest(s.ctx, validated, s.rpcClient) + numObservations, err := s.handleReobservationRequest(s.ctx, validatedObservation, s.rpcClient) if err != nil { logger.Error("failed to process observation request", zap.Uint32("chainID", m.ChainId), @@ -698,7 +698,7 @@ func (s *SolanaWatcher) fetchBlock(ctx context.Context, logger *zap.Logger, slot } // processTransaction processes a transaction and publishes any Wormhole events. -func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.Client, tx *solana.Transaction, meta *rpc.TransactionMeta, slot uint64, validated *watchers.ValidObservation, isReobservation bool) (numObservations uint32) { +func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.Client, tx *solana.Transaction, meta *rpc.TransactionMeta, slot uint64, validatedObservation *watchers.ValidObservation, isReobservation bool) (numObservations uint32) { // SECURITY: Validate transaction metadata before accessing fields if metadataErr := validateTransactionMeta(meta); metadataErr != nil { if s.logger.Level().Enabled(zapcore.DebugLevel) { @@ -776,7 +776,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C } } } else { - found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, i, validated, isReobservation) + found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, i, validatedObservation, isReobservation) if err != nil { s.logger.Error("malformed Wormhole instruction", zap.Error(err), @@ -822,7 +822,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C } } } else { - found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, innerIdx, validated, isReobservation) + found, err := s.processInstruction(ctx, rpcClient, slot, inst, programIndex, tx, signature, innerIdx, validatedObservation, isReobservation) if err != nil { s.logger.Error("malformed Wormhole instruction", zap.Error(err), @@ -850,7 +850,7 @@ func (s *SolanaWatcher) processTransaction(ctx context.Context, rpcClient *rpc.C return } -func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.Client, slot uint64, inst solana.CompiledInstruction, programIndex uint16, tx *solana.Transaction, signature solana.Signature, idx int, validated *watchers.ValidObservation, isReobservation bool) (bool, error) { +func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.Client, slot uint64, inst solana.CompiledInstruction, programIndex uint16, tx *solana.Transaction, signature solana.Signature, idx int, validatedObservation *watchers.ValidObservation, isReobservation bool) (bool, error) { if inst.ProgramIDIndex != programIndex { return false, nil } @@ -904,15 +904,15 @@ func (s *SolanaWatcher) processInstruction(ctx context.Context, rpcClient *rpc.C } common.RunWithScissors(ctx, s.errC, "retryFetchMessageAccount", func(ctx context.Context) error { - s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, 0, validated, signature) + s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, 0, validatedObservation, signature) return nil }) return true, nil } -func (s *SolanaWatcher) retryFetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, retry uint, validated *watchers.ValidObservation, signature solana.Signature) { - _, retryable := s.fetchMessageAccount(ctx, rpcClient, acc, slot, validated, signature) +func (s *SolanaWatcher) retryFetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, retry uint, validatedObservation *watchers.ValidObservation, signature solana.Signature) { + _, retryable := s.fetchMessageAccount(ctx, rpcClient, acc, slot, validatedObservation, signature) if retryable { if retry >= maxRetries { @@ -931,13 +931,13 @@ func (s *SolanaWatcher) retryFetchMessageAccount(ctx context.Context, rpcClient zap.Uint("retry", retry)) common.RunWithScissors(ctx, s.errC, "retryFetchMessageAccount", func(ctx context.Context) error { - s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, retry+1, validated, signature) + s.retryFetchMessageAccount(ctx, rpcClient, acc, slot, retry+1, validatedObservation, signature) return nil }) } } -func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, validated *watchers.ValidObservation, signature solana.Signature) (numObservations uint32, retryable bool) { +func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc.Client, acc solana.PublicKey, slot uint64, validatedObservation *watchers.ValidObservation, signature solana.Signature) (numObservations uint32, retryable bool) { // Fetching account rCtx, cancel := context.WithTimeout(ctx, rpcTimeout) defer cancel() @@ -993,7 +993,7 @@ func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc. zap.Binary("data", data)) } - observation, err := s.processMessageAccount(s.logger, data, acc, signature, validated != nil) + observation, err := s.processMessageAccount(s.logger, data, acc, signature, validatedObservation != nil) if err != nil { return 0, false } @@ -1001,8 +1001,8 @@ func (s *SolanaWatcher) fetchMessageAccount(ctx context.Context, rpcClient *rpc. return 0, false } - if validated != nil { - if err := s.PublishReobservation(*validated, observation); err != nil { + if validatedObservation != nil { + if err := s.PublishReobservation(*validatedObservation, observation); err != nil { s.logger.Error("failed to publish reobservation", zap.Error(err)) return 0, false } diff --git a/node/pkg/watchers/solana/reobserve.go b/node/pkg/watchers/solana/reobserve.go index ad8dcaf79da..7a438963692 100644 --- a/node/pkg/watchers/solana/reobserve.go +++ b/node/pkg/watchers/solana/reobserve.go @@ -67,9 +67,9 @@ func (s *SolanaWatcher) Reobserve(_ context.Context, chainID vaa.ChainID, txID [ s.logger.Info("received a request to reobserve using a custom endpoint", zap.Stringer("chainID", chainID), zap.Any("txID", txID), zap.String("url", customEndpoint)) rpcClient := rpc.New(customEndpoint) //nolint:contextcheck // See comment above for the reason why we don't use the passed in context. - validated, err := s.Validate(&gossipv1.ObservationRequest{ChainId: uint32(chainID), TxHash: txID}) + validatedObservation, err := s.Validate(&gossipv1.ObservationRequest{ChainId: uint32(chainID), TxHash: txID}) if err != nil { return 0, err } - return s.handleReobservationRequest(s.ctx, validated, rpcClient) + return s.handleReobservationRequest(s.ctx, validatedObservation, rpcClient) } diff --git a/node/pkg/watchers/solana/watcher_methods_test.go b/node/pkg/watchers/solana/watcher_methods_test.go index cc1d834b6b2..913bdc7e34c 100644 --- a/node/pkg/watchers/solana/watcher_methods_test.go +++ b/node/pkg/watchers/solana/watcher_methods_test.go @@ -39,13 +39,13 @@ func TestWatcherValidate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -74,7 +74,7 @@ func TestWatcherPublishMessage(t *testing.T) { } func TestWatcherPublishReobservation(t *testing.T) { - validated, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSolana), TxHash: make([]byte, SolanaAccountLen)}) + validatedObservation, err := newMethodTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSolana), TxHash: make([]byte, SolanaAccountLen)}) require.NoError(t, err) tests := []struct { name string @@ -87,7 +87,7 @@ func TestWatcherPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newMethodTestWatcher(msgC).PublishReobservation(validated, tt.msg) + err := newMethodTestWatcher(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return diff --git a/node/pkg/watchers/sui/watcher.go b/node/pkg/watchers/sui/watcher.go index 9554a159a47..09deab36c9a 100644 --- a/node/pkg/watchers/sui/watcher.go +++ b/node/pkg/watchers/sui/watcher.go @@ -268,12 +268,12 @@ func NewWatcher( } func (e *Watcher) Validate(req *gossipv1.ObservationRequest) (watchers.ValidObservation, error) { - validated, err := watchers.ValidateObservationRequest(req, e.chainID) + validatedObservation, err := watchers.ValidateObservationRequest(req, e.chainID) if err != nil { return watchers.ValidObservation{}, err } - return validated, nil + return validatedObservation, nil } func (e *Watcher) ChainID() vaa.ChainID { @@ -303,8 +303,8 @@ func (e *Watcher) PublishReobservation(observation watchers.ValidObservation, ms return e.PublishMessage(msg) } -func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiResult, validated *watchers.ValidObservation) error { - isReobservation := validated != nil +func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiResult, validatedObservation *watchers.ValidObservation) error { + isReobservation := validatedObservation != nil if body.ID.TxDigest == nil { return errors.New("missing TxDigest field") } @@ -379,21 +379,22 @@ func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiR // Verifies the observation through the Sui transaction verifier, if enabled, followed // by publishing the observation to the message channel. - if validated != nil { + var publishErr error + if validatedObservation != nil { if e.suiTxVerifier != nil { verifiedMsg, verifyErr := e.verify(ctx, observation, *body.ID.TxDigest, logger) if verifyErr != nil { - err = verifyErr + publishErr = verifyErr } else { observation = &verifiedMsg } } - if err == nil { - err = e.PublishReobservation(*validated, observation) + if publishErr == nil { + publishErr = e.PublishReobservation(*validatedObservation, observation) } - if err == nil { + if publishErr == nil { logger.Info("message observed", observation.ZapFields()...) return nil } @@ -401,24 +402,24 @@ func (e *Watcher) inspectBody(ctx context.Context, logger *zap.Logger, body SuiR if e.suiTxVerifier != nil { verifiedMsg, verifyErr := e.verify(ctx, observation, *body.ID.TxDigest, logger) if verifyErr != nil { - err = verifyErr + publishErr = verifyErr } else { observation = &verifiedMsg } } - if err == nil { - err = e.PublishMessage(observation) + if publishErr == nil { + publishErr = e.PublishMessage(observation) } - if err == nil { + if publishErr == nil { logger.Info("message observed", observation.ZapFields()...) } } - if err != nil { + if publishErr != nil { suiTransferVerifierFailures.Inc() logger.Error("Message publication error", zap.String("TxDigest", *body.ID.TxDigest), - zap.Error(err)) + zap.Error(publishErr)) } return nil @@ -523,13 +524,13 @@ func (e *Watcher) Run(ctx context.Context) error { logger.Error("sui_fetch_obvs_req context done") return ctx.Err() case r := <-e.obsvReqC: - validated, err := e.Validate(r) + validatedObservation, err := e.Validate(r) if err != nil { watchers.LogInvalidObservationRequest(logger, r, err) continue } - tx58 := base58.Encode(validated.TxHash()) + tx58 := base58.Encode(validatedObservation.TxHash()) payload := fmt.Sprintf(`{"jsonrpc":"2.0", "id": 1, "method": "sui_getEvents", "params": ["%s"]}`, tx58) @@ -567,9 +568,9 @@ func (e *Watcher) Run(ctx context.Context) error { } for i, chunk := range res.Result { - err := e.inspectBody(ctx, logger, chunk, &validated) - if err != nil { - logger.Info("sui_fetch_obvs_req skipping event data in result", zap.String("txhash", tx58), zap.Int("index", i), zap.Error(err)) + inspectErr := e.inspectBody(ctx, logger, chunk, &validatedObservation) + if inspectErr != nil { + logger.Info("sui_fetch_obvs_req skipping event data in result", zap.String("txhash", tx58), zap.Int("index", i), zap.Error(inspectErr)) } } } diff --git a/node/pkg/watchers/sui/watcher_methods_test.go b/node/pkg/watchers/sui/watcher_methods_test.go index 3614d93d4ff..e26e11d5ff8 100644 --- a/node/pkg/watchers/sui/watcher_methods_test.go +++ b/node/pkg/watchers/sui/watcher_methods_test.go @@ -38,13 +38,13 @@ func TestWatcherValidate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(tt.req) if tt.wantErr { require.Error(t, err) return } require.NoError(t, err) - assert.Equal(t, tt.req.GetTxHash(), validated.TxHash()) + assert.Equal(t, tt.req.GetTxHash(), validatedObservation.TxHash()) }) } } @@ -73,7 +73,7 @@ func TestWatcherPublishMessage(t *testing.T) { } func TestWatcherPublishReobservation(t *testing.T) { - validated, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}) + validatedObservation, err := newTestWatcher(make(chan *common.MessagePublication, 1)).Validate(&gossipv1.ObservationRequest{ChainId: uint32(vaa.ChainIDSui), TxHash: make([]byte, common.TxIDLenMin)}) require.NoError(t, err) tests := []struct { name string @@ -86,7 +86,7 @@ func TestWatcherPublishReobservation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { msgC := make(chan *common.MessagePublication, 1) - err := newTestWatcher(msgC).PublishReobservation(validated, tt.msg) + err := newTestWatcher(msgC).PublishReobservation(validatedObservation, tt.msg) if tt.wantErr { require.Error(t, err) return