BCF-2901 Fix potential JAID collisions in multi chain env (#11821)

* Fix potential JAID collisions in multi chain env This fixes some rest api responses getting omitted when the JAID was same across multiple chains * Add JAID constructor that formats JAID with chainID prefix * minor test fix * Flip FormatWithPrefixedChainID params to make more sense
smartcontractkit · Feb 12, 2024 · 1fb4203 · 1fb4203
1 parent c84f1b5
commit 1fb4203
Show file tree

Hide file tree

Showing 26 changed files with 275 additions and 32 deletions.
diff --git a/core/cmd/cosmos_node_commands_test.go b/core/cmd/cosmos_node_commands_test.go
@@ -48,8 +48,8 @@ func TestShell_IndexCosmosNodes(t *testing.T) {
 	nodes := *r.Renders[0].(*cmd.CosmosNodePresenters)
 	require.Len(t, nodes, 1)
 	n := nodes[0]
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(chainID, *node.Name), n.ID)
 	assert.Equal(t, chainID, n.ChainID)
-	assert.Equal(t, *node.Name, n.ID)
 	assert.Equal(t, *node.Name, n.Name)
 	wantConfig, err := toml.Marshal(node)
 	require.NoError(t, err)

diff --git a/core/cmd/evm_node_commands_test.go b/core/cmd/evm_node_commands_test.go
@@ -60,13 +60,13 @@ func TestShell_IndexEVMNodes(t *testing.T) {
 	n1 := nodes[0]
 	n2 := nodes[1]
 	assert.Equal(t, chainID.String(), n1.ChainID)
-	assert.Equal(t, *node1.Name, n1.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(chainID.String(), *node1.Name), n1.ID)
 	assert.Equal(t, *node1.Name, n1.Name)
 	wantConfig, err := toml.Marshal(node1)
 	require.NoError(t, err)
 	assert.Equal(t, string(wantConfig), n1.Config)
 	assert.Equal(t, chainID.String(), n2.ChainID)
-	assert.Equal(t, *node2.Name, n2.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(chainID.String(), *node2.Name), n2.ID)
 	assert.Equal(t, *node2.Name, n2.Name)
 	wantConfig2, err := toml.Marshal(node2)
 	require.NoError(t, err)

diff --git a/core/cmd/ocr2_keys_commands_test.go b/core/cmd/ocr2_keys_commands_test.go
@@ -32,7 +32,7 @@ func TestOCR2KeyBundlePresenter_RenderTable(t *testing.T) {
 	pubKeyConfig := key.ConfigEncryptionPublicKey()
 	pubKey := key.OffchainPublicKey()
 	p := cmd.OCR2KeyBundlePresenter{
-		JAID: cmd.JAID{ID: bundleID},
+		JAID: cmd.NewJAID(bundleID),
 		OCR2KeysBundleResource: presenters.OCR2KeysBundleResource{
 			JAID:              presenters.NewJAID(key.ID()),
 			ChainType:         "evm",

diff --git a/core/cmd/solana_node_commands_test.go b/core/cmd/solana_node_commands_test.go
@@ -55,13 +55,13 @@ func TestShell_IndexSolanaNodes(t *testing.T) {
 	n1 := nodes[0]
 	n2 := nodes[1]
 	assert.Equal(t, id, n1.ChainID)
-	assert.Equal(t, *node1.Name, n1.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(id, *node1.Name), n1.ID)
 	assert.Equal(t, *node1.Name, n1.Name)
 	wantConfig, err := toml.Marshal(node1)
 	require.NoError(t, err)
 	assert.Equal(t, string(wantConfig), n1.Config)
 	assert.Equal(t, id, n2.ChainID)
-	assert.Equal(t, *node2.Name, n2.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(id, *node2.Name), n2.ID)
 	assert.Equal(t, *node2.Name, n2.Name)
 	wantConfig2, err := toml.Marshal(node2)
 	require.NoError(t, err)

diff --git a/core/cmd/starknet_node_commands_test.go b/core/cmd/starknet_node_commands_test.go
@@ -54,13 +54,13 @@ func TestShell_IndexStarkNetNodes(t *testing.T) {
 	n1 := nodes[0]
 	n2 := nodes[1]
 	assert.Equal(t, id, n1.ChainID)
-	assert.Equal(t, *node1.Name, n1.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(id, *node1.Name), n1.ID)
 	assert.Equal(t, *node1.Name, n1.Name)
 	wantConfig, err := toml.Marshal(node1)
 	require.NoError(t, err)
 	assert.Equal(t, string(wantConfig), n1.Config)
 	assert.Equal(t, id, n2.ChainID)
-	assert.Equal(t, *node2.Name, n2.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(id, *node2.Name), n2.ID)
 	assert.Equal(t, *node2.Name, n2.Name)
 	wantConfig2, err := toml.Marshal(node2)
 	require.NoError(t, err)

diff --git a/core/internal/cltest/cltest.go b/core/internal/cltest/cltest.go
@@ -165,6 +165,10 @@ func MustRandomBytes(t *testing.T, l int) (b []byte) {
 	return b
 }
 
+func FormatWithPrefixedChainID(chainID, id string) string {
+	return fmt.Sprintf("%s/%s", chainID, id)
+}
+
 type JobPipelineV2TestHelper struct {
 	Prm pipeline.ORM
 	Jrm job.ORM

diff --git a/core/web/eth_keys_controller.go b/core/web/eth_keys_controller.go
@@ -270,7 +270,7 @@ func (ekc *ETHKeysController) Chain(c *gin.Context) {
 		jsonAPIError(c, http.StatusBadRequest, errors.Errorf("invalid address: %s, must be hex address", keyID))
 		return
 	}
-	address := common.HexToAddress((keyID))
+	address := common.HexToAddress(keyID)
 
 	cid := c.Query("evmChainID")
 	chain, ok := ekc.getChain(c, cid)

diff --git a/core/web/eth_keys_controller_test.go b/core/web/eth_keys_controller_test.go
@@ -284,7 +284,8 @@ func TestETHKeysController_ChainSuccess_UpdateNonce(t *testing.T) {
 	err := cltest.ParseJSONAPIResponse(t, resp, &updatedKey)
 	assert.NoError(t, err)
 
-	assert.Equal(t, key.ID(), updatedKey.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(cltest.FixtureChainID.String(), key.Address.String()), updatedKey.ID)
+	assert.Equal(t, key.Address.String(), updatedKey.Address)
 	assert.Equal(t, cltest.FixtureChainID.String(), updatedKey.EVMChainID.String())
 	assert.Equal(t, false, updatedKey.Disabled)
 }
@@ -328,7 +329,8 @@ func TestETHKeysController_ChainSuccess_Disable(t *testing.T) {
 	err := cltest.ParseJSONAPIResponse(t, resp, &updatedKey)
 	assert.NoError(t, err)
 
-	assert.Equal(t, key.ID(), updatedKey.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(updatedKey.EVMChainID.String(), key.Address.String()), updatedKey.ID)
+	assert.Equal(t, key.Address.String(), updatedKey.Address)
 	assert.Equal(t, cltest.FixtureChainID.String(), updatedKey.EVMChainID.String())
 	assert.Equal(t, true, updatedKey.Disabled)
 }
@@ -371,7 +373,8 @@ func TestETHKeysController_ChainSuccess_Enable(t *testing.T) {
 	err := cltest.ParseJSONAPIResponse(t, resp, &updatedKey)
 	assert.NoError(t, err)
 
-	assert.Equal(t, key.ID(), updatedKey.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(cltest.FixtureChainID.String(), key.Address.String()), updatedKey.ID)
+	assert.Equal(t, key.Address.String(), updatedKey.Address)
 	assert.Equal(t, cltest.FixtureChainID.String(), updatedKey.EVMChainID.String())
 	assert.Equal(t, false, updatedKey.Disabled)
 }
@@ -436,7 +439,8 @@ func TestETHKeysController_ChainSuccess_ResetWithAbandon(t *testing.T) {
 	err = cltest.ParseJSONAPIResponse(t, resp, &updatedKey)
 	assert.NoError(t, err)
 
-	assert.Equal(t, key.ID(), updatedKey.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(cltest.FixtureChainID.String(), key.Address.String()), updatedKey.ID)
+	assert.Equal(t, key.Address.String(), updatedKey.Address)
 	assert.Equal(t, cltest.FixtureChainID.String(), updatedKey.EVMChainID.String())
 	assert.Equal(t, false, updatedKey.Disabled)
 
@@ -663,7 +667,8 @@ func TestETHKeysController_DeleteSuccess(t *testing.T) {
 	err := cltest.ParseJSONAPIResponse(t, resp, &deletedKey)
 	assert.NoError(t, err)
 
-	assert.Equal(t, key0.ID(), deletedKey.ID)
+	assert.Equal(t, cltest.FormatWithPrefixedChainID(cltest.FixtureChainID.String(), key0.Address.String()), deletedKey.ID)
+	assert.Equal(t, key0.Address.String(), deletedKey.Address)
 	assert.Equal(t, cltest.FixtureChainID.String(), deletedKey.EVMChainID.String())
 	assert.Equal(t, false, deletedKey.Disabled)
 

diff --git a/core/web/presenters/chain_msg_test.go b/core/web/presenters/chain_msg_test.go
@@ -0,0 +1,69 @@
+package presenters
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/manyminds/api2go/jsonapi"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/smartcontractkit/chainlink/v2/core/internal/testutils/cosmostest"
+	"github.com/smartcontractkit/chainlink/v2/core/internal/testutils/solanatest"
+)
+
+func TestSolanaMessageResource(t *testing.T) {
+	id := "1"
+	chainID := solanatest.RandomChainID()
+	r := NewSolanaMsgResource(id, chainID)
+	assert.Equal(t, chainID, r.ChainID)
+
+	b, err := jsonapi.Marshal(r)
+	require.NoError(t, err)
+
+	expected := fmt.Sprintf(`
+	{
+	   "data":{
+		  "type":"solana_messages",
+		  "id":"%s/%s",
+		  "attributes":{
+			 "ChainID":"%s",
+			 "from":"",
+			 "to":"",
+			 "amount":0
+		  }
+	   }
+	}
+	`, chainID, id, chainID)
+
+	assert.JSONEq(t, expected, string(b))
+}
+
+func TestCosmosMessageResource(t *testing.T) {
+	id := "1"
+	chainID := cosmostest.RandomChainID()
+	contractID := "cosmos1p3ucd3ptpw902fluyjzkq3fflq4btddac9sa3s"
+	r := NewCosmosMsgResource(id, chainID, contractID)
+	assert.Equal(t, chainID, r.ChainID)
+	assert.Equal(t, contractID, r.ContractID)
+
+	b, err := jsonapi.Marshal(r)
+	require.NoError(t, err)
+
+	expected := fmt.Sprintf(`
+	{
+	   "data":{
+		  "type":"cosmos_messages",
+		  "id":"%s/%s",
+		  "attributes":{
+			 "ChainID":"%s",
+			 "ContractID":"%s",
+			 "State":"",
+			 "TxHash":null
+		  }
+	   }
+	}
+	`, chainID, id, chainID, contractID)
+
+	assert.JSONEq(t, expected, string(b))
+}
diff --git a/core/web/presenters/cosmos_chain.go b/core/web/presenters/cosmos_chain.go
@@ -36,7 +36,7 @@ func (r CosmosNodeResource) GetName() string {
 // NewCosmosNodeResource returns a new CosmosNodeResource for node.
 func NewCosmosNodeResource(node types.NodeStatus) CosmosNodeResource {
 	return CosmosNodeResource{NodeResource{
-		JAID:    NewJAID(node.Name),
+		JAID:    NewPrefixedJAID(node.Name, node.ChainID),
 		ChainID: node.ChainID,
 		Name:    node.Name,
 		State:   node.State,

diff --git a/core/web/presenters/cosmos_msg.go b/core/web/presenters/cosmos_msg.go
@@ -17,7 +17,7 @@ func (CosmosMsgResource) GetName() string {
 // NewCosmosMsgResource returns a new partial CosmosMsgResource.
 func NewCosmosMsgResource(id string, chainID string, contractID string) CosmosMsgResource {
 	return CosmosMsgResource{
-		JAID:       NewJAID(id),
+		JAID:       NewPrefixedJAID(id, chainID),
 		ChainID:    chainID,
 		ContractID: contractID,
 	}

diff --git a/core/web/presenters/eth_key.go b/core/web/presenters/eth_key.go
@@ -40,7 +40,7 @@ type NewETHKeyOption func(*ETHKeyResource)
 // Use the functional options to inject the ETH and LINK balances
 func NewETHKeyResource(k ethkey.KeyV2, state ethkey.State, opts ...NewETHKeyOption) *ETHKeyResource {
 	r := &ETHKeyResource{
-		JAID:        NewJAID(k.Address.Hex()),
+		JAID:        NewPrefixedJAID(k.Address.Hex(), state.EVMChainID.String()),
 		EVMChainID:  state.EVMChainID,
 		Address:     k.Address.Hex(),
 		EthBalance:  nil,

diff --git a/core/web/presenters/eth_key_test.go b/core/web/presenters/eth_key_test.go
@@ -55,7 +55,7 @@ func TestETHKeyResource(t *testing.T) {
 	{
 		"data":{
 		   "type":"eTHKeys",
-		   "id":"%s",
+		   "id":"42/%s",
 		   "attributes":{
 			  "address":"%s",
 			  "evmChainID":"42",
@@ -84,7 +84,7 @@ func TestETHKeyResource(t *testing.T) {
 	{
 		"data": {
 			"type":"eTHKeys",
-			"id":"%s",
+			"id":"42/%s",
 			"attributes":{
 				"address":"%s",
 			  	"evmChainID":"42",

diff --git a/core/web/presenters/eth_tx.go b/core/web/presenters/eth_tx.go
@@ -66,6 +66,7 @@ func NewEthTxResourceFromAttempt(txa txmgr.TxAttempt) EthTxResource {
 
 	if txa.Tx.ChainID != nil {
 		r.EVMChainID = *big.New(txa.Tx.ChainID)
+		r.JAID = NewPrefixedJAID(r.JAID.ID, txa.Tx.ChainID.String())
 	}
 
 	if tx.Sequence != nil {

diff --git a/core/web/presenters/eth_tx_test.go b/core/web/presenters/eth_tx_test.go
@@ -20,12 +20,14 @@ import (
 func TestEthTxResource(t *testing.T) {
 	t.Parallel()
 
+	chainID := big.NewInt(54321)
 	tx := txmgr.Tx{
 		ID:             1,
 		EncodedPayload: []byte(`{"data": "is wilding out"}`),
 		FromAddress:    common.HexToAddress("0x1"),
 		ToAddress:      common.HexToAddress("0x2"),
 		FeeLimit:       uint32(5000),
+		ChainID:        chainID,
 		State:          txmgrcommon.TxConfirmed,
 		Value:          big.Int(assets.NewEthValue(1)),
 	}
@@ -52,7 +54,7 @@ func TestEthTxResource(t *testing.T) {
 			"sentAt": "",
 			"to": "0x0000000000000000000000000000000000000002",
 			"value": "0.000000000000000001",
-			"evmChainID": "0"
+			"evmChainID": "54321"
 		  }
 		}
 	  }
@@ -85,7 +87,7 @@ func TestEthTxResource(t *testing.T) {
 	{
 		"data": {
 		  "type": "evm_transactions",
-		  "id": "0x0000000000000000000000000000000000000000000000000000000000010203",
+		  "id": "54321/0x0000000000000000000000000000000000000000000000000000000000010203",
 		  "attributes": {
 			"state": "confirmed",
 			"data": "0x7b2264617461223a202269732077696c64696e67206f7574227d",
@@ -98,7 +100,7 @@ func TestEthTxResource(t *testing.T) {
 			"sentAt": "300",
 			"to": "0x0000000000000000000000000000000000000002",
 			"value": "0.000000000000000001",
-			"evmChainID": "0"
+			"evmChainID": "54321"
 		  }
 		}
 	  }

diff --git a/core/web/presenters/evm_chain.go b/core/web/presenters/evm_chain.go
@@ -34,7 +34,7 @@ func (r EVMNodeResource) GetName() string {
 // NewEVMNodeResource returns a new EVMNodeResource for node.
 func NewEVMNodeResource(node types.NodeStatus) EVMNodeResource {
 	return EVMNodeResource{NodeResource{
-		JAID:    NewJAID(node.Name),
+		JAID:    NewPrefixedJAID(node.Name, node.ChainID),
 		ChainID: node.ChainID,
 		Name:    node.Name,
 		State:   node.State,

diff --git a/core/web/presenters/evm_forwarder_test.go b/core/web/presenters/evm_forwarder_test.go
@@ -0,0 +1,64 @@
+package presenters
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/manyminds/api2go/jsonapi"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/forwarders"
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/utils"
+	"github.com/smartcontractkit/chainlink/v2/core/chains/evm/utils/big"
+)
+
+func TestEVMForwarderResource(t *testing.T) {
+	var (
+		ID        = int64(1)
+		address   = utils.RandomAddress()
+		chainID   = *big.NewI(4)
+		createdAt = time.Now()
+		updatedAt = time.Now().Add(time.Second)
+	)
+	fwd := forwarders.Forwarder{
+		ID:         ID,
+		Address:    address,
+		EVMChainID: chainID,
+		CreatedAt:  createdAt,
+		UpdatedAt:  updatedAt,
+	}
+
+	r := NewEVMForwarderResource(fwd)
+	assert.Equal(t, fmt.Sprint(ID), r.ID)
+	assert.Equal(t, address, r.Address)
+	assert.Equal(t, chainID, r.EVMChainID)
+	assert.Equal(t, createdAt, r.CreatedAt)
+	assert.Equal(t, updatedAt, r.UpdatedAt)
+
+	b, err := jsonapi.Marshal(r)
+	require.NoError(t, err)
+
+	createdAtMarshalled, err := createdAt.MarshalText()
+	require.NoError(t, err)
+	updatedAtMarshalled, err := updatedAt.MarshalText()
+	require.NoError(t, err)
+
+	expected := fmt.Sprintf(`
+	{
+	   "data":{
+		  "type":"evm_forwarder",
+		  "id":"%d",
+		  "attributes":{
+			 "address":"%s",
+			 "evmChainId":"%s",
+			 "createdAt":"%s",
+			 "updatedAt":"%s"
+		  }
+	   }
+	}
+	`, ID, strings.ToLower(address.String()), chainID.String(), string(createdAtMarshalled), string(updatedAtMarshalled))
+	assert.JSONEq(t, expected, string(b))
+}
diff --git a/core/web/presenters/jsonapi.go b/core/web/presenters/jsonapi.go
@@ -1,6 +1,7 @@
 package presenters
 
 import (
+	"fmt"
 	"strconv"
 )
 
@@ -14,6 +15,11 @@ func NewJAID(id string) JAID {
 	return JAID{id}
 }
 
+// NewPrefixedJAID prefixes JAID with chain id in %s/%s format.
+func NewPrefixedJAID(id string, chainID string) JAID {
+	return JAID{ID: fmt.Sprintf("%s/%s", chainID, id)}
+}
+
 // NewJAIDInt32 converts an int32 into a JAID
 func NewJAIDInt32(id int32) JAID {
 	return JAID{strconv.Itoa(int(id))}