search

movementlabsxyz / movement

The Movement Network is a Move-based L2 on Ethereum.
Apache License 2.0
72 stars 61 forks source link

Error in Suzuka + bridge `setup` since merge of 561 #703

Open andygolay opened 1 week ago

andygolay commented 1 week ago

Describe the bug

Suzuka + bridge build, run with command

CELESTIA_LOG_LEVEL=FATAL CARGO_PROFILE=release CARGO_PROFILE_FLAGS=--release  nix develop --extra-experimental-features nix-command --extra-experimental-features flakes --command bash  -c "just bridge native build.setup.eth-local.celestia-local.bridge --keep-tui"

fails since the merge of 651 (commit 7aa85be210524d0b18c1f29299e4f3be8778d13b Merge pull request #561 from movementlabsxyz/primata/contract-pipeline)

See screenshot for error:

image

This highlights the need to utilize proper CI checks when merging PRs.

To Reproduce

At the movement project root:

git reset --hard 249228ad58f31bd9e0f492d83b9396baff72e77c

You should see

HEAD is now at 249228ad update: client calls with match arms

This is the commit prior to the merge of 651.

Expected behavior This command should start the bridge without errors:

CELESTIA_LOG_LEVEL=FATAL CARGO_PROFILE=release CARGO_PROFILE_FLAGS=--release  nix develop --extra-experimental-features nix-command --extra-experimental-features flakes --command bash  -c "just bridge native build.setup.eth-local.celestia-local.bridge --keep-tui"

Screenshots Expected behavior screenshot:

image
0xmovses commented 1 week ago

If we had our new e2e running on cicd:bridge job, this would have caught the issue before merging. This effectively stalls e2e testing on the bridge. What I would suggest is a dedicated PR just to add the bridge e2e test in CI (at the moment it's just integration tests).

Then revert this PR, and re-merge it with it having to pass that test.

@l-monninger @0xPrimata please chime in on what you think the appropriate approach is here.