C chain integ tests #204

cam-schultz · 2023-12-18T23:07:03Z

Why this should be merged

Fixes #193

TODO:

Modify run.sh to use Warp enabled C-Chain once use snapshop compatible with cchain warp avalanche-cli#1330 is merged and a release issued
Add back in example bash script, and expand README with usage
~~Add ethclient interface that both subnet-evm and coreth ethclient implement~~
- Tracking in Design unified ethclient interface #208

How this works

Replaces one of the chains in each of the E2E test cases with the C-Chain.

How this was tested

E2E test modifications, CI

How is this documented

N/A

cam-schultz · 2023-12-18T23:07:50Z

docker/relayerConfigTemplate.json

@@ -5,7 +5,7 @@
    "source-subnets": [
        {
            "subnet-id": "11111111111111111111111111111111LpoYY",
-            "chain-id": "<C_CHAIN_CHAIN_ID>",
+            "blockchain-id": "<C_CHAIN_CHAIN_ID>",


TODO: Update the AWM Relayer version in versions.sh once ava-labs/icm-services#115 is merged and a new release created.

Nit, should we renaming the template variables <BLOCKCHAIN_ID> rather than <CHAIN_ID>?

cam-schultz · 2023-12-19T02:12:05Z

scripts/local/e2e_test.sh

@@ -16,6 +16,7 @@ BASEDIR=${BASEDIR:-"$HOME/.teleporter-deps"}

 cwd=$(pwd)
 # Install the avalanchego and subnet-evm binaries
+rm -rf $BASEDIR/avalanchego


A recent change is causing the network artifacts to not fully be cleaned on exit, causing errors on subsequent runs. This works around that.

cam-schultz · 2023-12-19T02:13:37Z

tests/local/e2e_test.go

 	ginkgo.It("Send native tokens from subnet A to B and back", func() {
-		flows.NativeTokenBridge(localNetworkInstance)
+		flows.NativeTokenBridge(LocalNetworkInstance)
 	})
 	ginkgo.It("Send ERC20 tokens from subnet A to Native tokens on subnet B and back", func() {
-		flows.ERC20ToNativeTokenBridge(localNetworkInstance)
+		flows.ERC20ToNativeTokenBridge(LocalNetworkInstance)
 	})


These two test cases currently fail when using the C-Chain on one end. I need to investigate this further.

Fixed in #207

cam-schultz · 2023-12-19T02:13:56Z

tests/local/network.go

@@ -54,22 +59,19 @@ const (
 	fundedKeyStr = "56289e99c94b6912bfc12adc093c9b51124f0dc54ac7a766b2bc5ccf558d8027"
 )

-func newLocalNetwork(warpGenesisFile string) *localNetwork {
+func NewLocalNetwork(warpGenesisFile string) *LocalNetwork {


We export these functions so that they can be used in awm-relayer

michaelkaplan13

Few small suggestions, but other looking good 👍

michaelkaplan13 · 2023-12-19T22:07:59Z

docker/relayerConfigTemplate.json

@@ -5,7 +5,7 @@
    "source-subnets": [
        {
            "subnet-id": "11111111111111111111111111111111LpoYY",
-            "chain-id": "<C_CHAIN_CHAIN_ID>",
+            "blockchain-id": "<C_CHAIN_CHAIN_ID>",


Nit, should we renaming the template variables <BLOCKCHAIN_ID> rather than <CHAIN_ID>?

michaelkaplan13 · 2023-12-19T22:12:00Z

tests/local/network.go

-	subnetNodeNames                 map[ids.ID][]string
+type LocalNetwork struct {
+	teleporterContractAddress common.Address
+	primaryNetworkInfo        *interfaces.SubnetTestInfo


I think cchainInfo, etc is probably more accurate for the naming through out these tests, because the SubnetTestInfo doesn't have any value regarding the X or P chains.

While true, I'd prefer to keep the naming consistent with respect to subnet vs blockchain for all subnets, including the primary network.

Hm, fair point. The previous naming assumption was that a given subnet will only have 1 blockchain, which is still true for the subnets, but not true for primary network.

I'm fine with it for now, but would prefer we clean it up at some point...maybe BlockchainTestInfo for the struct name, and subnetBlockchainInfo rather than subnetsInfo (along with cChainInfo) to make everything fully accurate/consistent?

I'm also curious now if we need subnetAID and subnetBID, and if so if we should also add primaryNetworkID here, but can also be deferred to later clean up IMO.

michaelkaplan13 · 2023-12-19T22:14:33Z

tests/local/network.go

+	nodeURIs = append(nodeURIs, n.subnetsInfo[n.subnetAID].NodeURIs...)
+	nodeURIs = append(nodeURIs, n.subnetsInfo[n.subnetBID].NodeURIs...)


Do you know if there are any other nodes in the local primary network other than the subnet validator nodes?

If not, could you add a comment explaining that all nodes should be accounted for here?

Nope, I believe these should be all the nodes. I added a comment noting such.

For context, we only ever use the 1st element of this list, so the actual length and content is pretty arbitrary.

michaelkaplan13 · 2023-12-19T22:15:56Z

tests/local/network.go

+	subnets := n.GetSubnetsInfo()
+	subnets = append(subnets, *n.primaryNetworkInfo)


Maybe worth considering something along the lines of a network.GetAllBlockchainInfo helper for cases like this.

Good call, added

minghinmatthewlam · 2023-12-21T05:34:37Z

tests/flows/block_hash_publish_receive.go

 	"github.com/ava-labs/subnet-evm/accounts/abi/bind"
 	"github.com/ava-labs/teleporter/tests/interfaces"
 	"github.com/ava-labs/teleporter/tests/utils"
 	. "github.com/onsi/gomega"
 )

 func BlockHashPublishReceive(network interfaces.Network) {
-	subnetAInfo, subnetBInfo, _ := utils.GetThreeSubnets(network)
+	subnetAInfo := network.GetPrimaryNetworkInfo()


nit: can you rename these to primarySubnetInfo or similar

same for other test cases

I think it would be better if the tests were agnostic as to which network is which, but we have to do some work to unify the interfaces between the core-eth and subnet-evm clients.

"Primary Network" is actually the correct terminology: https://docs.avax.network/learn/avalanche/avalanche-platform

tests/local/network.go

docker/run_setup.sh

scripts/local/examples/basic_send_receive.sh

geoff-vball · 2023-12-21T13:02:34Z

tests/flows/block_hash_publish_receive.go

 	"github.com/ava-labs/subnet-evm/accounts/abi/bind"
 	"github.com/ava-labs/teleporter/tests/interfaces"
 	"github.com/ava-labs/teleporter/tests/utils"
 	. "github.com/onsi/gomega"
 )

 func BlockHashPublishReceive(network interfaces.Network) {
-	subnetAInfo, subnetBInfo, _ := utils.GetThreeSubnets(network)
+	subnetAInfo := network.GetPrimaryNetworkInfo()


I think it would be better if the tests were agnostic as to which network is which, but we have to do some work to unify the interfaces between the core-eth and subnet-evm clients.

michaelkaplan13 · 2023-12-21T16:28:34Z

Other than the small suggestions from Geoff and Matt, LGTM.

Co-authored-by: minghinmatthewlam <[email protected]> Signed-off-by: cam-schultz <[email protected]>

…into c-chain-integ-tests

geoff-vball

👍

cam-schultz added 12 commits December 15, 2023 22:05

handle primary network case in relayer

d8767f4

remove subnet C

c34b3d8

expect 2 subnets

e6f7a3e

basic send receive c chain

db7ea7d

clean up data dir

78ee57a

most tests working w/c-chain

1360541

block hash test working c-chain

3958785

remove dummy test

06253fc

export LocalNetwork

25eec62

export TearDownNetwork

52e5f0c

update relayer config

1012306

latest avalanche-cli

e147529

cam-schultz commented Dec 18, 2023

View reviewed changes

cam-schultz commented Dec 19, 2023

View reviewed changes

cam-schultz self-assigned this Dec 19, 2023

cam-schultz marked this pull request as ready for review December 19, 2023 02:22

cam-schultz requested review from michaelkaplan13, minghinmatthewlam, gwen917, geoff-vball and bernard-avalabs as code owners December 19, 2023 02:22

cam-schultz added 3 commits December 19, 2023 21:12

testnet primary network

368cc91

fix block hash publisher c-chain test case

1f89c95

remove redundant var

40da608

michaelkaplan13 reviewed Dec 19, 2023

View reviewed changes

cam-schultz added 3 commits December 20, 2023 15:14

replace chainid with blockchainid

6263664

add comment

f03cfab

add getAllSubnetsInfo helper

37ff640

minghinmatthewlam reviewed Dec 21, 2023

View reviewed changes

tests/local/network.go Outdated Show resolved Hide resolved

minghinmatthewlam reviewed Dec 21, 2023

View reviewed changes

tests/local/network.go Outdated Show resolved Hide resolved

minghinmatthewlam reviewed Dec 21, 2023

View reviewed changes

tests/local/network.go Show resolved Hide resolved

geoff-vball reviewed Dec 21, 2023

View reviewed changes

cam-schultz and others added 7 commits December 21, 2023 16:47

c-chain specifier const

c311907

Update tests/flows/block_hash_publish_receive.go

3a6a683

Co-authored-by: minghinmatthewlam <[email protected]> Signed-off-by: cam-schultz <[email protected]>

comments

fe7bd6c

stream c chain logs

b497678

remove references to unused vars

793ae1f

Merge branch 'c-chain-integ-tests' of github.com:ava-labs/teleporter …

d8214da

…into c-chain-integ-tests

echo blockchain id

826fa59

cam-schultz requested review from minghinmatthewlam and geoff-vball December 21, 2023 16:54

geoff-vball previously approved these changes Dec 21, 2023

View reviewed changes

cam-schultz added 3 commits December 21, 2023 19:59

update avalanche-cli

fdab992

typo

838e9b7

example script interacts with c-chain

d38c26d

cam-schultz dismissed geoff-vball’s stale review via d38c26d December 21, 2023 20:01

cam-schultz added 2 commits December 21, 2023 20:19

comment command

8a38f6e

awm relayer v0.2.7

3f8b6bf

michaelkaplan13 previously approved these changes Dec 21, 2023

View reviewed changes

cam-schultz added 2 commits December 21, 2023 20:56

fix example script

43a8d3e

cleanup docker setup readme

87d3efd

cam-schultz dismissed michaelkaplan13’s stale review via 87d3efd December 21, 2023 20:56

geoff-vball approved these changes Dec 21, 2023

View reviewed changes

michaelkaplan13 approved these changes Dec 21, 2023

View reviewed changes

cam-schultz merged commit 9e3880d into main Dec 21, 2023
15 checks passed

cam-schultz deleted the c-chain-integ-tests branch December 21, 2023 21:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C chain integ tests #204

C chain integ tests #204

cam-schultz commented Dec 18, 2023 •

edited

Loading

cam-schultz Dec 18, 2023

michaelkaplan13 Dec 19, 2023

cam-schultz Dec 19, 2023

cam-schultz Dec 19, 2023

cam-schultz Dec 19, 2023

cam-schultz Dec 19, 2023

michaelkaplan13 left a comment

michaelkaplan13 Dec 19, 2023

michaelkaplan13 Dec 19, 2023

cam-schultz Dec 20, 2023

michaelkaplan13 Dec 21, 2023

michaelkaplan13 Dec 19, 2023

cam-schultz Dec 20, 2023

michaelkaplan13 Dec 19, 2023

cam-schultz Dec 20, 2023

minghinmatthewlam Dec 21, 2023

minghinmatthewlam Dec 21, 2023

geoff-vball Dec 21, 2023

cam-schultz Dec 21, 2023

geoff-vball Dec 21, 2023

michaelkaplan13 commented Dec 21, 2023

geoff-vball left a comment

		nodeURIs = append(nodeURIs, n.subnetsInfo[n.subnetAID].NodeURIs...)
		nodeURIs = append(nodeURIs, n.subnetsInfo[n.subnetBID].NodeURIs...)

		subnets := n.GetSubnetsInfo()
		subnets = append(subnets, *n.primaryNetworkInfo)

C chain integ tests #204

C chain integ tests #204

Conversation

cam-schultz commented Dec 18, 2023 • edited Loading

Why this should be merged

How this works

How this was tested

How is this documented

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkaplan13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkaplan13 commented Dec 21, 2023

geoff-vball left a comment

Choose a reason for hiding this comment

cam-schultz commented Dec 18, 2023 •

edited

Loading