Change ChainDescriptions into blobs by bart-linera · Pull Request #3787 · linera-io/linera-protocol

bart-linera · 2025-04-16T20:00:10Z

Motivation

This will allow for easier initialization of the network at genesis, among other things.

Proposal

ChainDescription will become a blob, and OpenChain messages will be removed. Calling open_chain will create the blob, and the chain will actually get initialized when a block is proposed for it.

Test Plan

Should be covered by CI.

Release Plan

Nothing to do / These changes follow the usual release cycle.

Links

linera-base/src/data_types.rs

linera-base/src/identifiers.rs

afck · 2025-04-17T09:02:24Z

linera-core/src/client/mod.rs

+                    RemoteNode::download_blobs(&blob_ids, validators, self.blob_download_timeout)
+                        .await
+                        .ok_or(LocalNodeError::BlobsNotFound(blob_ids))?;
+                self.local_node.storage_client().write_blobs(&blobs).await?;


But now the remote node could have tricked us, and the chain might not actually have been created.

I think we need to use something like ChainClient::update_local_node_with_blobs_from, which downloads proof from the validators that the blob was created. Except if we already have at least one correctly signed certificate from this chain—its first block can also count as a proof, I guess (if we double-check the committee information with the admin chain).

Hmm, good point 🤔

This one I'd consider a blocker, or at least it should have a GitHub issue and be addressed right after this PR.

afck · 2025-04-17T09:05:57Z

linera-core/src/client/mod.rs

+        let mut validators_vec = validators.iter().collect::<Vec<_>>();
+        validators_vec.shuffle(&mut rand::thread_rng());
+        for remote_node in validators_vec {
+            let info = self.chain_info(chain_id, validators).await?;


Actually, at this point we only need the next block height. Why not just set it to zero if we don't have the chain at all?

We also need to return the info if target_next_block_height is zero, so we need to get it. (Maybe we could do a shortcut if it's not zero, but I'm not sure if it's worth it...)

afck · 2025-04-17T09:11:58Z

linera-core/src/client/mod.rs

+        let blob = RemoteNode::download_blob(&nodes, chain_desc_id, self.blob_download_timeout)
+            .await
+            .ok_or(LocalNodeError::BlobsNotFound(vec![chain_desc_id]))?;
+        self.local_node.storage_client().write_blob(&blob).await?;


Same as above: We can't trust that blob yet.

(Also kind of a blocker.)

linera-core/src/client/mod.rs

linera-core/src/remote_node.rs

linera-chain/src/chain.rs

linera-chain/src/block.rs

linera-base/src/identifiers.rs

linera-base/src/data_types.rs

afck · 2025-04-17T09:55:07Z

linera-base/src/data_types.rs

+pub struct ChainDescription {
+    origin: ChainOrigin,
+    timestamp: Timestamp,
+    config: OpenChainConfig,


We should consider inlining this type here. (Probably in a later PR.)

ma2bd · 2025-04-16T20:09:34Z

linera-chain/src/chain.rs


    /// Invariant for the states of active chains.
-    pub fn ensure_is_active(&self) -> Result<(), ChainError> {
+    pub async fn ensure_is_active(&mut self, local_time: Timestamp) -> Result<(), ChainError> {


nit: maybe due for a rename

ma2bd · 2025-04-16T20:11:32Z

linera-chain/src/data_types.rs

-
-    /// Rearranges the messages in the bundle so that the first message is an `OpenChain` message.
-    /// Returns whether the `OpenChain` message was found at all.
-    pub fn put_openchain_at_front(bundles: &mut [IncomingBundle]) -> bool {


linera-base/src/data_types.rs

afck · 2025-04-28T16:04:48Z

examples/hex-game/tests/hex_game.rs

+        .into_iter()
+        .filter_map(|(blob_id, blob)| {
+            (blob_id.blob_type == BlobType::ChainDescription)
+                .then(|| bcs::from_bytes::<ChainDescription>(blob.content().bytes()).unwrap())


Check the operation results instead, and make the operation return the child chain ID as a result? (Also below.)

linera-base/src/ownership.rs

linera-chain/src/chain.rs

linera-chain/src/unit_tests/chain_tests.rs

afck · 2025-04-28T16:32:23Z

linera-client/src/client_context.rs

@@ -432,28 +468,22 @@ where
        };

        // Download the parent chain.


~~We don't have to do that at all anymore, I think.~~

(Edit: I guess we do, as a proof for the chain description blob.)

I actually removed it 😅 My thinking is that the proof will be downloaded in ensure_has_chain_description if we don't have the relevant blob (of course, currently it doesn't check the proof, but will be fixed as a part of #2351 ).

linera-client/src/client_context.rs

afck · 2025-04-28T16:37:16Z

linera-core/src/chain_worker/state/attempted_changes.rs

@@ -303,13 +303,7 @@ where
        }
        let local_time = self.state.storage.clock().current_time();
        // TODO(#2351): This sets the committee and then checks that committee's signatures.


(Right… that's still the case now, is it?)

Yes, I think it is 😬

afck · 2025-04-28T16:39:14Z

linera-core/src/client/mod.rs

+                    RemoteNode::download_blobs(&blob_ids, validators, self.blob_download_timeout)
+                        .await
+                        .ok_or(LocalNodeError::BlobsNotFound(blob_ids))?;
+                self.local_node.storage_client().write_blobs(&blobs).await?;


This one I'd consider a blocker, or at least it should have a GitHub issue and be addressed right after this PR.

afck · 2025-04-28T16:41:14Z

linera-core/src/client/mod.rs

+        let blob = RemoteNode::download_blob(&nodes, chain_desc_id, self.blob_download_timeout)
+            .await
+            .ok_or(LocalNodeError::BlobsNotFound(vec![chain_desc_id]))?;
+        self.local_node.storage_client().write_blob(&blob).await?;


(Also kind of a blocker.)

afck · 2025-04-28T16:44:45Z

linera-storage/src/lib.rs

-        balance: Amount,
-        timestamp: Timestamp,
-    ) -> Result<(), ChainError>
+    async fn create_chain(&self, description: ChainDescription) -> Result<(), ChainError>


Not sure if this really belongs in linera-storage. Maybe this function should just be inlined at the call site(s) now?

There are quite a few call sites, so I think it still makes sense to have such a function - but I agree that it seems to be a bit outside of storage's scope. I'm not sure how to clean it up best right now, though.

linera-service/src/linera/net_up_utils.rs

linera-service/src/linera/main.rs

afck · 2025-04-28T16:54:17Z

linera-service/src/linera/main.rs

+            };
+            let admin_chain_description =
+                ChainDescription::new(origin, admin_config.clone(), timestamp);
+            let admin_id = admin_chain_description.id();


Some of this is probably duplicated somewhere else in the initialization code (where we actually write the description). Maybe it can be simplified.

You're probably right, but I don't see an obvious way of doing this now - I'll take a note of it for later.

linera-execution/src/system.rs

linera-base/src/data_types.rs

linera-sdk/src/test/validator.rs

linera-execution/tests/test_execution.rs

linera-core/src/unit_tests/worker_tests.rs

afck · 2025-04-29T08:33:07Z

linera-core/src/unit_tests/worker_tests.rs

-{
-    init_worker_with_chains(storage, [(description, owner, balance)]).await
-}
+    async fn add_child_chain(


Do we ever actually do that? Insert a child chain just deus ex machina, without an OpenChain operation?

afck · 2025-04-29T08:38:55Z

linera-core/src/unit_tests/worker_tests.rs

-        action: MessageAction::Accept,
-    };
+    let description = env
+        .add_child_chain(chain_2_desc.id(), sender_key_pair.public().into(), balance)


I see… but now this test makes less sense because there's no OpenChain message. (And the name is wrong, too.) I'd almost say this test can be removed. And then maybe we don't need add_child_chain anymore.

It's also used in one more test to simulate creating a block that opens a chain, without actually using it. So while we could remove this test, this function would still have some use.

afck · 2025-04-29T08:43:39Z

linera-execution/src/runtime.rs

    /// Controller to track fuel and storage consumption.
    resource_controller: ResourceController,
+    /// Additional context for the runtime.
+    user_context: UserInstance::UserContext,


Is it really worth creating that trait and associated type for this? We already have several optional fields in here that are None in some cases (operation vs. message vs. service call etc.).

I'm not sure, honestly, but it does avoid some awkward unwrapping or returning errors from codepaths that should be unreachable.
I might even have started with an option and then decided that this is better, but I don't remember if that was how I introduced this, TBH 😬

afck · 2025-04-29T08:46:58Z

linera-execution/src/runtime.rs

-                next_message_id: message_id,
+                parent_id,
+                block_height,
+                timestamp,


And the timestamp is actually not necessary here, because it's part of the system execution state.

I'm not sure what you mean 😅 I believe we need this to initialize the new system execution state with the correct timestamp (and it might be different than the one stored in the current chain's system execution state until now), or am I missing something?

This is while executing the block on the parent chain, is it? Specifically, executing the OpenChain operation?

The timestamp we want to assign to the child chain is the timestamp of the current block. And yes, that is exactly the timestamp in the system execution state, because we update that at the beginning of block execution.

Ah, I see! That makes sense. I'll try to apply your suggestion, then - could be that it will let me remove this whole trait thing 👍

afck

Approving, since as discussed the comments can also be addressed in follow-up PRs.

incoming bundle

…ored in ChainDescription

…_certificates_from_validator

afck · 2025-04-30T11:58:04Z

linera-service/tests/linera_net_tests.rs

    // Since the faucet chain exceeds the configured maximum length, the faucet should have
    // switched after the first new chain.
-    assert!(other_outcome.message_id.chain_id != outcome.message_id.chain_id);
+    assert!(other_outcome.chain_id != outcome.chain_id);


This should compare the parent chain IDs. I'll fix this when I port #3863 to the main branch.

Oops, you're right! 😬 Thanks!

…end (#3870) ## Motivation After #3787, it is no longer needed to initialize the shard of the dual-store databases separately. ## Proposal Remove special code added in #2734 ## Test Plan CI

@bart-linera

This is a completion of @bart-linera's #4075: ## Motivation #3787 left a potential security issue: when a client was downloading a missing `ChainDescription` blob, it would just download a blob, but wouldn't make sure that it was legitimately created on another chain. ## Proposal Whenever a `ChainDescription` is fetched, fetch the certificate for the block that created it and validate it against committees known from the admin chain. ## Test Plan The only occurrences of `RemoteNode::download_blob[s]` are now after `process_certificate` returned `BlobsNotFound`, i.e. after checking a certificate's signatures. CI should catch regressions. ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - Closes #2351 - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist) ## Notes The `TODO` in `attempted_changes.rs` has been removed after verifying that the safety issue is no longer present there: the committees are taken from a blob, but that blob can only exist on the validator if it has been legitimately created on another chain. --------- Co-authored-by: Bartłomiej Kamiński <bartlomiej.kaminski@linera.io>

## Motivation We simulate a range of validator faults in the `linera-core` tests. One of them is called `Malicious`, but it really just initializes the storage with wrong chains and returns `ArithmeticError`s in some cases. After #3787 (comment), the wrong initialization even causes the chains to have different chain ID, not just a wrong balance, so they will often return `InactiveChain` errors. Even before that chain, the storage initialization fault didn't play well with `set_fault_type`: Even if we switch them to `Honest`, they will still be faulty due to their storage contents. ## Proposal Replace `Malicious` with `NoChains`: Storage _does_ get initialized, but these validators return `InactiveChain` anyway, when handling block proposals, certificates or chain info queries. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. - (But might as well be backported: It's test only and not problematic.) ## Links - Closes #3858. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)

## Motivation We simulate a range of validator faults in the `linera-core` tests. One of them is called `Malicious`, but it really just initializes the storage with wrong chains and returns `ArithmeticError`s in some cases. After linera-io#3787 (comment), the wrong initialization even causes the chains to have different chain ID, not just a wrong balance, so they will often return `InactiveChain` errors. Even before that chain, the storage initialization fault didn't play well with `set_fault_type`: Even if we switch them to `Honest`, they will still be faulty due to their storage contents. ## Proposal Replace `Malicious` with `NoChains`: Storage _does_ get initialized, but these validators return `InactiveChain` anyway, when handling block proposals, certificates or chain info queries. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. - (But might as well be backported: It's test only and not problematic.) ## Links - Closes linera-io#3858. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)

## Motivation We simulate a range of validator faults in the `linera-core` tests. One of them is called `Malicious`, but it really just initializes the storage with wrong chains and returns `ArithmeticError`s in some cases. After #3787 (comment), the wrong initialization even causes the chains to have different chain ID, not just a wrong balance, so they will often return `InactiveChain` errors. Even before that chain, the storage initialization fault didn't play well with `set_fault_type`: Even if we switch them to `Honest`, they will still be faulty due to their storage contents. ## Proposal Replace `Malicious` with `NoChains`: Storage _does_ get initialized, but these validators return `InactiveChain` anyway, when handling block proposals, certificates or chain info queries. ## Test Plan CI ## Release Plan - Nothing to do / These changes follow the usual release cycle. - (But might as well be backported: It's test only and not problematic.) ## Links - Closes #3858. - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)

ma2bd reviewed Apr 16, 2025

View reviewed changes

linera-base/src/data_types.rs Show resolved Hide resolved

ma2bd reviewed Apr 16, 2025

View reviewed changes

linera-base/src/data_types.rs Outdated Show resolved Hide resolved

ma2bd reviewed Apr 16, 2025

View reviewed changes

linera-base/src/identifiers.rs Outdated Show resolved Hide resolved

afck reviewed Apr 17, 2025

View reviewed changes

bart-linera force-pushed the issue-2390 branch from ab76ddd to f59a50e Compare April 18, 2025 14:21

ma2bd reviewed Apr 18, 2025

View reviewed changes

bart-linera force-pushed the issue-2390 branch 10 times, most recently from f69e2ed to 0de904d Compare April 28, 2025 16:38

bart-linera marked this pull request as ready for review April 28, 2025 16:39

afck reviewed Apr 28, 2025

View reviewed changes

linera-base/src/data_types.rs Outdated Show resolved Hide resolved

afck reviewed Apr 29, 2025

View reviewed changes

bart-linera mentioned this pull request Apr 29, 2025

Don't set the committee before checking it in process_confirmed_block. #2351

Closed

afck approved these changes Apr 29, 2025

View reviewed changes

bart-linera mentioned this pull request Apr 29, 2025

Rethink faulty validators in tests #3858

Closed

bart-linera added 6 commits April 29, 2025 19:25

Change ChainDescriptions into blobs

57a532c

Improve some error messages

b02a208

Ensure the sender's ChainDescription is present when processing an

6263749

incoming bundle

Fix examples

5599845

Attempt to fix remote net tests

714b055

Address cosmetic review comments

79b39de

bart-linera added 6 commits April 29, 2025 19:26

Fix the greatgrandchild remote net test

3462549

Separate the config passed to the OpenChain operation from the one st…

21149bc

…ored in ChainDescription

Make sure that all relevant blobs are present in synchronize_received…

43f74e5

…_certificates_from_validator

Temporarily ignore intermittently failing test_finalize_validated

813892d

Fix errors after rebasing

9c1abef

Address some review comments

3e63591

bart-linera force-pushed the issue-2390 branch from 97ea4ff to 3e63591 Compare April 29, 2025 17:29

Small fixes

595c93b

bart-linera merged commit 49c09a9 into linera-io:main Apr 29, 2025
26 checks passed

bart-linera deleted the issue-2390 branch April 29, 2025 18:54

afck reviewed Apr 30, 2025

View reviewed changes

ma2bd mentioned this pull request May 1, 2025

Simplification storage initialization of the dual-store database backend #3870

Merged

bart-linera mentioned this pull request Jun 6, 2025

Make sure that blobs downloaded by the client are validated #4075

Closed

afck mentioned this pull request Jun 9, 2025

Make sure that blobs downloaded by the client are validated #4082

Merged

afck mentioned this pull request Sep 2, 2025

Replace fault type Malicious with NoChains. #4462

Merged

Conversation

bart-linera commented Apr 16, 2025

Motivation

Proposal

Test Plan

Release Plan

Links

Uh oh!

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!

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!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

afck Apr 28, 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!

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!

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!

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!

Uh oh!

Uh oh!

Uh oh!

afck Apr 28, 2025 •

edited

Loading