feat: Da sequencer, add state propagation by musitdev · Pull Request #1226 · movementlabsxyz/movement

musitdev · 2025-05-09T14:39:06Z

Summary

RFCs: Link to RFC, Link to RFC, or $\emptyset$.
Categories: any of protocol-units, networks, scripts, util, cicd, or misc.

Implementation of the issue #1222

Some commits are already in feature branch because I've used squash and merge in the other PR instead of simple merge, so as this branch start from the other PR branch, git can't re-associate them.
The setup and test part is already merged in feature branch.

Add state propagation from a main node to all other full node.
Full node validate the received state with the one they have at the same height.
If it diverges the node is stopped.
The propagation use the da-sequencer grpc service.

Start updating the follower node setup / migration doc update.

Changelog

Add a new entry point to da sequencer grpc: send_state
Add a property in node config to define the main node.
Main node send state after execution.
All full node store the last 100 state they receive.
All full node validate the state after execution with the one they have received.
Full node installation doc creation.

Testing

Unit test of the state validation:

 cargo test -p movement-full-node test_validate_state

e2e test:

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

Outstanding issues

…1087) Co-authored-by: Andreas Penzkofer <36140574+apenzk@users.noreply.github.com> Co-authored-by: Mikhail Zabaluev <mikhail.zabaluev@movementlabs.xyz> Co-authored-by: Liam Monninger <79056955+l-monninger@users.noreply.github.com> Co-authored-by: Andy Golay <andygolay@gmail.com> Co-authored-by: musitdev <philippe.delrieu@free.fr> Co-authored-by: Richard Melkonian <35300528+0xmovses@users.noreply.github.com> Co-authored-by: primata <primata@movementlabs.xyz> Co-authored-by: Richard Melkonian <r.v.melkonian@gmail.com> Co-authored-by: Radu Popa <radupopa21be@gmail.com> Co-authored-by: Liam Monninger <l.mak.monninger@gmail.com>

"Scenarios per client", removed the "number_" prefix which was a handful to type and didn't add clarity.

ganymedio · 2025-05-09T19:22:42Z

@musitdev the unit test passes for me, but for the e2e test, I'm wondering, is expected, for setup?:

I was able to run the test, and it looks like some scenarios failed. I'll DM you the run logs.

l-monninger · 2025-05-12T00:41:11Z

-					)
-					.await
+			tokio::spawn({
+				let da_signer = self


@musitdev I think it might be better to move these into a config struct for the exec_settle_task at this point. Would you like for me to drop that in a PR?

Yes, if you want. As I say later, it's still a test to understand better why state of node changes. So it can be removed depending on the cause of the state divergence.

Okay, I will drop something tomorrow.

l-monninger · 2025-05-12T00:43:42Z

@@ -0,0 +1,117 @@
+use maptos_opt_executor::executor::ExecutionState;


Add module comments for this.

Ok, I do that. I'll put the explanation that I'm going to write to your next question.

l-monninger · 2025-05-12T00:47:41Z

+    bytes signature = 2;
+}
+
+message MainNodeState {


This makes more sense as LeaderNodeState. Why the change to Main*?

Leader* was never meant to refer to nodes that gossip state to to Follower*. That was a misconception that you guys decided to advance, additionally shutting down the transaction posting from followers.

The Leader* produces blocks as well as executing. The Follower* receives blocks and executes them.

Yes—propagating a state isn’t our goal. I added this feature because we’ve seen edge cases that cause nodes to diverge. Without on‐chain consensus, we need a “source of truth” node to decide which state is correct. I’ve called it the main node (to avoid confusing it with the leader node, which has a different responsibility). The main node’s only job is to broadcast its state; it has no other special functions.

Originally, this was just a test harness to uncover divergence scenarios. I could have used ad-hoc debug RPCs, but I decided to build a permanent, robust solution. Even after we find and fix specific bugs, we can’t guarantee they’ll never reappear. As partners rely more on their follower nodes—and as network load increases—early detection of divergence becomes critical. We’ll use this implementation first to detect divergence, then decide whether to keep it long-term.

We disabled transaction submission on follower nodes because our current DA light-node can’t handle Celestia back-pressure when multiple batches arrive simultaneously. On mainnet we discovered partners were unintentionally using their follower nodes: Celestia blobs backed up in the client, and transactions took minutes to include in a block.

The new DA-Sequencer architecture changes this:

The DA-Sequencer produces and publishes blocks.

Full nodes handle everything else (RPC, execution, indexing, gRPC, etc.).

l-monninger · 2025-05-12T00:54:57Z

 use tracing::info;

+#[derive(Debug, Clone)]
+pub struct ExecutionState {


movement/util/movement-types/src/block.rs

Line 175 in a537dc0

#[derive(Serialize, Deserialize, Clone, Default, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]

BlockCommitment already has a height and an id. Commitments would match on timestamp and version too. What's the need for adding this separate struct versus including it in BlockCommitment?

They don't share the same use case. Change on one struct shouldn't affect the other. They must be separated.

Maybe explain briefly why they must be separated as a doc comment /// above the struct.

I add a comment.

Copilot

Pull Request Overview

This PR implements state propagation from the main node to full nodes, adding a new gRPC endpoint (send_state), configuration for the main node, and verification of state consistency between nodes.

Introduces a state verifier module to store and validate node states.
Enhances task execution in the DA sequencer client to propagate execution state and verify it.
Updates configuration and deployment scripts to support the new state propagation mechanism.

Reviewed Changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
networks/movement/movement-full-node/src/node/tasks/state_verifier.rs	Adds a lightweight state verifier used to track and validate a limited history of node states.
networks/movement/movement-full-node/src/node/tasks/execute_settle.rs	Updates the block processing logic to include validation of state propagation, as well as spawning task to send new state to the DA sequencer.
networks/movement/movement-full-node/src/node/tasks/mod.rs	Registers the new state verifier module.
networks/movement/movement-full-node/src/node/partial.rs	Updates task spawning to supply new configuration options related to state propagation.
Cargo.toml, shell scripts, docs	Reflect configuration and dependency updates supporting the new state propagation functionality.

0xmovses

The unit tests and e2e test ran successfully. Output for e2e`movement-tests-load-tests was :

 Finished `dev` profile [unoptimized + debuginfo] target(s) in 2m 54s
     Running `target/debug/movement-tests-e2e-load-alice-bob`
  2025-05-12T13:53:16.421964Z  INFO movement_tests_e2e_load_alice_bob: final balances, Alice: 99999998400, Bob: 200
    at networks/movement/movement-client/src/bin/e2e/load_alice_bob.rs:162

  2025-05-12T13:53:16.423368Z  INFO movement_tests_e2e_load_alice_bob: final balances, Alice: 99999998400, Bob: 200
    at networks/movement/movement-client/src/bin/e2e/load_alice_bob.rs:162

  2025-05-12T13:53:16.425542Z  INFO movement_tests_e2e_load_alice_bob: final balances, Alice: 99999998400, Bob: 200
    at networks/movement/movement-client/src/bin/e2e/load_alice_bob.rs:162

  2025-05-12T13:53:16.425668Z  INFO movement_tests_e2e_load_alice_bob: final balances, Alice: 99999998400, Bob: 200
    at networks/movement/movement-client/src/bin/e2e/load_alice_bob.rs:162

  2025-05-12T13:53:16.426223Z  INFO movement_tests_e2e_load_alice_bob: final balances, Alice: 99999998400, Bob: 200
    at networks/movement/movement-client/src/bin/e2e/load_alice_bob.rs:162

  2025-05-12T13:53:16.427455Z  INFO movement_tests_e2e_load_alice_bob: final balances, Alice: 99999998400, Bob: 200
    at networks/movement/movement-client/src/bin/e2e/load_alice_bob.rs:162

  2025-05-12T13:53:16.427598Z  INFO movement_tests_e2e_load_alice_bob: final balances, Alice: 99999998400, Bob: 200
    at networks/movement/movement-client/src/bin/e2e/load_alice_bob.rs:162

  2025-05-12T13:53:16.428009Z  INFO movement_tests_e2e_load_alice_bob: final

I've left comments around style, implementation questions and doc improvements

0xmovses · 2025-05-12T14:03:10Z

+						.map_err(|_| anyhow::anyhow!("Invalid main_node_verifying_key key"))
+				})
+		})
+		.transpose()?;


Seems quite unwieldily just to get the verify_key. If we expect this verifying_key to be accessed elsewhere, abstract the map() call into some method on main_node_verifying_key.

0xmovses · 2025-05-12T14:05:25Z

+			tracing::warn!("Bad node state data, no state in it.");
+			return Ok(tonic::Response::new(BatchWriteResponse { answer: false }));
+		}
+		let state = state_data.state.unwrap(); //unwrap tested before.


//unwrap tested before. where, how? Maybe a more detailed comment about why its ok to unwrap() here, as I don't know what it is and would rather it be out.

It's just the statement before:

if state_data.state.is_none() {

If you prefer I can to a match; perhaps it's more clear. I change it to a match.

Honestly, I liked anyhow::Context best here.

let state = state_data.context("state data should not be None")?;

0xmovses · 2025-05-12T14:08:27Z

 use tracing::info;

+#[derive(Debug, Clone)]
+pub struct ExecutionState {


Maybe explain briefly why they must be separated as a doc comment /// above the struct.

ganymedio · 2025-05-12T15:18:10Z

The updated E2E test

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

has passed for me:

Pending resolution of @l-monninger and @0xmovses change requests, I think this is good to merge.

Co-authored-by: Richard Melkonian <35300528+0xmovses@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…/movement into musitdev/propagate_state

l-monninger · 2025-05-13T12:55:25Z

+			Some(url) => url.to_string_lossy().into_owned(),
+			None => "http://movement-da-sequencer:30730/".to_string(),
+		};
+		tracing::info!("updating da-sequecner connection url with:{new_url}");


0xmovses

0xmovses and others added 30 commits April 7, 2025 18:32

feat: create initial structure for mock e2e DA

a80c3ac

fix" update configs

57c5c28

fix: process-compose and script definitions

9cde662

chore: remove old mock file

0f0cbe6

fix: process-compose test mock da

af71380

fix: config

93ad41b

fix: tweak config

5363b54

start the whole stack with process compose

395e9c9

remove scaffold script that are not used

e9b47aa

merge main and correct conflicts

f17de67

remove test files

e71df88

correct iniital Da height value

e693557

correct transaction pipe test

f79337c

feat: CI check cargo-fmt-check and cargo fmt fixes (#1197)

7c4d7ee

increase depends on waiting time for da-sequencer

669754c

test: improve logging in load test

f40183f

chore: rename test config fields and vars

88d8186

"Scenarios per client", removed the "number_" prefix which was a handful to type and didn't add clarity.

chore: rename and gitignore test logs

6ea9757

Beanch, update test and do some perf changes.

205054d

pull origin and correct conflicts

bc1f8ee

add some logs

84e94b6

change main task pulling. Select seem to always trigger, for test

66ce055

test spawn blockcing

5348d48

update load test

e16e66a

put all block execution in one sync block

9d4989f

change load soak test parameters

035dd5a

merge and resolve conflict

ab7b818

set load test local

df38cc0

add docker scripts

a35638a

correct build

014466d

musitdev requested review from l-monninger and sebtomba May 9, 2025 14:39

musitdev requested review from 0xmovses and ganymedio as code owners May 9, 2025 14:39

Merge branch 'feature/da-sequencer-node' into musitdev/propagate_state

1a64510

musitdev self-assigned this May 9, 2025

musitdev added the cicd:movement-containers label May 9, 2025

add state verification at the Da reception

ec6539a

l-monninger suggested changes May 12, 2025

View reviewed changes

correct chain id issue for load and soak test

e149347

0xmovses reviewed May 12, 2025

View reviewed changes

Comment thread docs/movement-node/run-fullnode/README.md Outdated

0xmovses requested a review from Copilot May 12, 2025 13:22

Copilot AI reviewed May 12, 2025

View reviewed changes

Comment thread networks/movement/movement-full-node/src/node/tasks/state_verifier.rs

Comment thread networks/movement/movement-full-node/src/node/tasks/execute_settle.rs Outdated

0xmovses suggested changes May 12, 2025

View reviewed changes

musitdev and others added 3 commits May 13, 2025 10:23

Apply suggestions from code review

a359027

Co-authored-by: Richard Melkonian <35300528+0xmovses@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

review corrections

f0e2c27

Merge branch 'musitdev/propagate_state' of github.com:movementlabsxyz…

cd65a36

…/movement into musitdev/propagate_state

musitdev requested review from 0xmovses and l-monninger May 13, 2025 10:01

musitdev added 2 commits May 13, 2025 13:52

review corrections

1ed20ae

add da connection url management for the setup migration

ff302aa

l-monninger suggested changes May 13, 2025

View reviewed changes

update installation script to define da connection url.

7b59c81

0xmovses approved these changes May 13, 2025

View reviewed changes

ganymedio approved these changes May 13, 2025

View reviewed changes

0xmovses merged commit 56b16b3 into feature/da-sequencer-node May 13, 2025
37 of 38 checks passed

musitdev deleted the musitdev/propagate_state branch June 13, 2025 15:34

		@@ -0,0 +1,117 @@
		use maptos_opt_executor::executor::ExecutionState;

Conversation

musitdev commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changelog

Testing

Outstanding issues

Uh oh!

ganymedio commented May 9, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

musitdev May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

0xmovses left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ganymedio commented May 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xmovses left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

musitdev commented May 9, 2025 •

edited

Loading

musitdev May 12, 2025 •

edited

Loading