瀏覽代碼

notary: fix bug where non-Ethereum messages were always Approved (#4577)

* notary: fix bug where non-Ethereum messages were always Approved
* add environment checks and workarounds for unit test and mock environments
John Saigle 1 周之前
父節點
當前提交
ee26e052ee
共有 2 個文件被更改,包括 82 次插入15 次删除
  1. 30 14
      node/pkg/notary/notary.go
  2. 52 1
      node/pkg/notary/notary_test.go

+ 30 - 14
node/pkg/notary/notary.go

@@ -38,6 +38,7 @@ import (
 
 	"github.com/certusone/wormhole/node/pkg/common"
 	"github.com/certusone/wormhole/node/pkg/db"
+	"github.com/certusone/wormhole/node/pkg/txverifier"
 	"github.com/wormhole-foundation/wormhole/sdk"
 	"github.com/wormhole-foundation/wormhole/sdk/vaa"
 
@@ -154,30 +155,45 @@ func (n *Notary) ProcessMsg(msg *common.MessagePublication) (v Verdict, err erro
 
 	n.logger.Debug("notary: processing message", msg.ZapFields()...)
 
-	// NOTE: Only token transfers originated on Ethereum are currently considered.
 	// For the initial implementation, the Notary only rules on messages based
 	// on the Transfer Verifier. However, there is no technical barrier to
 	// supporting other message types.
-	if msg.EmitterChain != vaa.ChainIDEthereum {
-		n.logger.Debug("notary: automatically approving message publication because it is not from Ethereum", msg.ZapFields()...)
+	if !txverifier.IsSupported(msg.EmitterChain) {
+		n.logger.Debug("notary: automatically approving message: sent from a chain without a transfer verifier implementation", msg.ZapFields()...)
 		return Approve, nil
 	}
 
 	if !vaa.IsTransfer(msg.Payload) {
-		n.logger.Debug("notary: automatically approving message publication because it is not a token transfer", msg.ZapFields()...)
+		n.logger.Debug("notary: automatically approving message: it is not a wrapped token transfer", msg.ZapFields()...)
 		return Approve, nil
 	}
 
-	if tokenBridge, ok := sdk.KnownTokenbridgeEmitters[msg.EmitterChain]; !ok {
-		// Return Unknown if the token bridge is not registered in the SDK.
-		n.logger.Error("notary: unknown token bridge emitter", msg.ZapFields()...)
-		return Unknown, errors.New("unknown token bridge emitter")
-	} else {
-		// Approve if the token transfer is not from the token bridge.
-		// For now, the notary only rules on token transfers from the token bridge.
-		if !bytes.Equal(msg.EmitterAddress.Bytes(), tokenBridge) {
-			n.logger.Debug("notary: automatically approving message publication because it is not from the token bridge", msg.ZapFields()...)
-			return Approve, nil
+	var tbEmitters = make(map[vaa.ChainID][]byte)
+	switch n.env {
+	case common.MainNet:
+		tbEmitters = sdk.KnownTokenbridgeEmitters
+	case common.TestNet:
+		tbEmitters = sdk.KnownTestnetTokenbridgeEmitters
+	case common.UnsafeDevNet:
+		tbEmitters = sdk.KnownDevnetTokenbridgeEmitters
+	case common.AccountantMock, common.GoTest:
+	default:
+		n.logger.Debug("skipping token bridge emitter check because environment is not mainnet or testnet")
+	}
+
+	// Perform emitter checks when outside of unit tests or mock environments
+	if n.env == common.MainNet || n.env == common.TestNet || n.env == common.UnsafeDevNet {
+		if tokenBridge, ok := tbEmitters[msg.EmitterChain]; !ok {
+			// Return Unknown if the token bridge is not registered in the SDK.
+			n.logger.Error("notary: unknown token bridge emitter", msg.ZapFields()...)
+			return Unknown, errors.New("unknown token bridge emitter")
+		} else {
+			// Approve if the token transfer is not from the token bridge.
+			// For now, the notary only rules on token transfers from the token bridge.
+			if !bytes.Equal(msg.EmitterAddress.Bytes(), tokenBridge) {
+				n.logger.Debug("notary: automatically approving message publication because it is not from the token bridge", msg.ZapFields()...)
+				return Approve, nil
+			}
 		}
 	}
 

+ 52 - 1
node/pkg/notary/notary_test.go

@@ -48,6 +48,57 @@ func makeTestNotary(t *testing.T) *Notary {
 	}
 }
 
+// TestNotary_AlwaysApproveNonTransferVerifierEmitters tests that all messages are approve if the emitter chain does not have a transfer verifier.
+// This test can be removed if the Notary is extended to support other chains.
+func TestNotary_AlwaysApproveNonTransferVerifierEmitters(t *testing.T) {
+	// NOTE: Solana does not have a transfer verifier implementation
+	tests := map[string]struct {
+		verificationState common.VerificationState
+		emitterChain      vaa.ChainID
+		verdict           Verdict
+	}{
+		"approve non-transfer verifier when Rejected": {
+			common.Rejected,
+			vaa.ChainIDSolana,
+			Approve,
+		},
+		"approve non-transfer verifier when Anomalous": {
+			common.Anomalous,
+			vaa.ChainIDSolana,
+			Approve,
+		},
+		"delay non-Ethereum messages for chain with transfer verifier when Rejected": {
+			common.Rejected,
+			vaa.ChainIDSepolia,
+			Delay,
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			n := makeTestNotary(t)
+			msg := makeUniqueMessagePublication(t)
+
+			// Set the emitter address to the known token bridge address for the environment.
+			msg.EmitterChain = test.emitterChain
+
+			err := msg.SetVerificationState(test.verificationState)
+			require.NoError(t, err)
+
+			require.True(t, vaa.IsTransfer(msg.Payload))
+
+			verdict, err := n.ProcessMsg(msg)
+			require.NoError(t, err)
+			require.Equal(
+				t,
+				test.verdict,
+				verdict,
+				fmt.Sprintf("verificationState=%s verdict=%s", msg.VerificationState().String(), verdict.String()),
+			)
+		})
+	}
+}
+
 func TestNotary_ProcessMessageCorrectVerdict(t *testing.T) {
 
 	// NOTE: This test should be exhaustive over VerificationState variants.
@@ -485,7 +536,7 @@ func makeUniqueMessagePublication(t *testing.T) *common.MessagePublication {
 	require.NoError(t, err)
 
 	// Required as the Notary checks the emitter address.
-	tokenBridge := sdk.KnownTokenbridgeEmitters[vaa.ChainIDEthereum]
+	tokenBridge := sdk.KnownDevnetTokenbridgeEmitters[vaa.ChainIDEthereum]
 	tokenBridgeAddress := vaa.Address(tokenBridge)
 	require.NoError(t, err)