ERC20 bridge multihop #140
Conversation
gwen917
requested review from
michaelkaplan13,
cam-schultz,
minghinmatthewlam,
geoff-vball and
bernard-avalabs
as code owners
| @@ -80,12 +82,15 @@ func SetupNetwork(warpGenesisFile string) { | |||
| // Name 10 new validators (which should have BLS key registered) | |||
| var subnetANodeNames []string | |||
| var subnetBNodeNames []string | |||
| for i := 1; i <= 10; i++ { | |||
| var subnetCNodeNames []string | |||
There was a problem hiding this comment.
I think validate_churn should be updated to issue txs to the propose vm for all subnets. Also the node ids collide from 15-19 right now
There was a problem hiding this comment.
I think I tried it from 16 to 20, but it still get error. I will double check.
There was a problem hiding this comment.
Here is failed CI with this change.
| @@ -51,6 +51,10 @@ var _ = ginkgo.BeforeSuite(func() { | |||
| var _ = ginkgo.AfterSuite(localUtils.TearDownNetwork) | |||
|
|
|||
| var _ = ginkgo.Describe("[Teleporter integration tests]", func() { | |||
| // Cross-chain application tests | |||
There was a problem hiding this comment.
after the comment below, and we issue tx to propose vm to all subnets, I'd like for validator churn to not need to be added last. Do you know right now if these tests are in random order?
There was a problem hiding this comment.
Agreed. If we can fix it we should. Otherwise what happens if we have another test that also needs to go last? 🙃
There was a problem hiding this comment.
instead of making the issue to proposevm calls to each subnet individually, can we iterate through all subnets in the network, get subnet info, and issue tx? To prevent in the future if we add more subnets for this test to break and need to be debugged.
| @@ -64,7 +68,4 @@ var _ = ginkgo.Describe("[Teleporter integration tests]", func() { | |||
| ginkgo.It("Resubmit altered message", ResubmitAlteredMessageGinkgo) | |||
| ginkgo.It("Relayer modifies message", RelayerModifiesMessageGinkgo) | |||
| ginkgo.It("Validator churn", ValidatorChurnGinkgo) | |||
There was a problem hiding this comment.
Can you add a comment here explaining that this test should be run last since it changes the network topology? In theory it shouldn't have any effect, but it seems like we have some further investigating to do. Can you please create a ticket and include all of your findings so far?
There was a problem hiding this comment.
LGTM, comment about checking total nodes instead of hardcoding is addressed in PR #93.
|
just bmping to create issue related to investigating |
Why this should be merged
Implements the following test cases from #27:
Port the integration test cases in erc20_bridge_multihop.sh into Gingko.
How this works
See above
How this was tested
New E2E test case
How is this documented