Skip to content

Commit

Permalink
Don't forget pending blobs if retrying at the next height. (#3242)
Browse files Browse the repository at this point in the history
## Motivation

This is a port of #3238 to the main branch.

## Proposal

Always set the pending blobs together with the pending block.

Also simplify some more blob-related client code.

## Test Plan

`test_re_propose_locked_block_with_blobs` was modified to re-propose a
`PublishDataBlob` operation. This fails without the fix.

## Release Plan

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

## Links

- Original PR: #3238 
- [reviewer
checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
  • Loading branch information
afck authored Feb 5, 2025
1 parent aa2a305 commit 1fc0922
Show file tree
Hide file tree
Showing 19 changed files with 302 additions and 323 deletions.
11 changes: 4 additions & 7 deletions linera-chain/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@
// Copyright (c) Zefchain Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

use std::{
collections::{BTreeSet, HashSet},
fmt::Debug,
};
use std::{collections::BTreeSet, fmt::Debug};

use async_graphql::SimpleObject;
use linera_base::{
Expand Down Expand Up @@ -482,7 +479,7 @@ impl Block {

/// Returns all the blob IDs required by this block.
/// Either as oracle responses or as published blobs.
pub fn required_blob_ids(&self) -> HashSet<BlobId> {
pub fn required_blob_ids(&self) -> BTreeSet<BlobId> {
let mut blob_ids = self.oracle_blob_ids();
blob_ids.extend(self.published_blob_ids());
blob_ids
Expand Down Expand Up @@ -512,8 +509,8 @@ impl Block {
}

/// Returns set of blob ids that were a result of an oracle call.
pub fn oracle_blob_ids(&self) -> HashSet<BlobId> {
let mut required_blob_ids = HashSet::new();
pub fn oracle_blob_ids(&self) -> BTreeSet<BlobId> {
let mut required_blob_ids = BTreeSet::new();
for responses in &self.body.oracle_responses {
for response in responses {
if let OracleResponse::Blob(blob_id) = response {
Expand Down
12 changes: 6 additions & 6 deletions linera-chain/src/certificate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod lite;
mod timeout;
mod validated;

use std::collections::HashSet;
use std::collections::BTreeSet;

pub use generic::GenericCertificate;
use linera_base::{
Expand Down Expand Up @@ -102,7 +102,7 @@ pub trait CertificateValue: Clone {

fn height(&self) -> BlockHeight;

fn required_blob_ids(&self) -> HashSet<BlobId>;
fn required_blob_ids(&self) -> BTreeSet<BlobId>;
}

impl CertificateValue for Timeout {
Expand All @@ -120,8 +120,8 @@ impl CertificateValue for Timeout {
self.height
}

fn required_blob_ids(&self) -> HashSet<BlobId> {
HashSet::new()
fn required_blob_ids(&self) -> BTreeSet<BlobId> {
BTreeSet::new()
}
}

Expand All @@ -140,7 +140,7 @@ impl CertificateValue for ValidatedBlock {
self.block().header.height
}

fn required_blob_ids(&self) -> HashSet<BlobId> {
fn required_blob_ids(&self) -> BTreeSet<BlobId> {
self.block().required_blob_ids()
}
}
Expand All @@ -160,7 +160,7 @@ impl CertificateValue for ConfirmedBlock {
self.block().header.height
}

fn required_blob_ids(&self) -> HashSet<BlobId> {
fn required_blob_ids(&self) -> BTreeSet<BlobId> {
self.block().required_blob_ids()
}
}
2 changes: 1 addition & 1 deletion linera-chain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ pub enum ChainExecutionContext {
Block,
}

trait ExecutionResultExt<T> {
pub trait ExecutionResultExt<T> {
fn with_execution_context(self, context: ChainExecutionContext) -> Result<T, ChainError>;
}

Expand Down
34 changes: 8 additions & 26 deletions linera-client/src/client_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,14 @@

#[cfg(with_testing)]
use std::num::NonZeroUsize;
use std::{
collections::{BTreeMap, HashSet},
sync::Arc,
};
use std::{collections::HashSet, sync::Arc};

use async_trait::async_trait;
use futures::Future;
use linera_base::{
crypto::KeyPair,
data_types::{Blob, BlockHeight, Timestamp},
identifiers::{Account, BlobId, ChainId},
data_types::{BlockHeight, Timestamp},
identifiers::{Account, ChainId},
ownership::ChainOwnership,
time::{Duration, Instant},
};
Expand Down Expand Up @@ -272,8 +269,7 @@ where
chain.block_hash,
chain.timestamp,
chain.next_block_height,
chain.pending_block.clone(),
chain.pending_blobs.clone(),
chain.pending_proposal.clone(),
);
chain_client.options_mut().message_policy = MessagePolicy::new(
self.blanket_message_policy,
Expand Down Expand Up @@ -326,19 +322,7 @@ where
key_pair: Option<KeyPair>,
timestamp: Timestamp,
) -> Result<(), Error> {
self.update_wallet_for_new_chain_internal(chain_id, key_pair, timestamp, BTreeMap::new())
.await
}

#[cfg(test)]
pub async fn update_wallet_for_new_chain_with_pending_blobs(
&mut self,
chain_id: ChainId,
key_pair: Option<KeyPair>,
timestamp: Timestamp,
pending_blobs: BTreeMap<BlobId, Blob>,
) -> Result<(), Error> {
self.update_wallet_for_new_chain_internal(chain_id, key_pair, timestamp, pending_blobs)
self.update_wallet_for_new_chain_internal(chain_id, key_pair, timestamp)
.await
}

Expand All @@ -347,7 +331,6 @@ where
chain_id: ChainId,
key_pair: Option<KeyPair>,
timestamp: Timestamp,
pending_blobs: BTreeMap<BlobId, Blob>,
) -> Result<(), Error> {
if self.wallet.get(chain_id).is_none() {
self.mutate_wallet(|w| {
Expand All @@ -357,8 +340,7 @@ where
block_hash: None,
timestamp,
next_block_height: BlockHeight::ZERO,
pending_block: None,
pending_blobs,
pending_proposal: None,
})
})
.await?;
Expand Down Expand Up @@ -644,7 +626,7 @@ where
.take(num_new_chains)
.collect();
let certificate = chain_client
.execute_operations(operations)
.execute_operations(operations, vec![])
.await?
.expect("should execute block with OpenChain operations");
let block = certificate.block();
Expand Down Expand Up @@ -713,7 +695,7 @@ where
// Put at most 1000 fungible token operations in each block.
for operations in operations.chunks(1000) {
chain_client
.execute_operations(operations.to_vec())
.execute_operations(operations.to_vec(), vec![])
.await?
.expect("should execute block with OpenChain operations");
}
Expand Down
8 changes: 3 additions & 5 deletions linera-client/src/unit_tests/chain_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

#![allow(clippy::large_futures)]

use std::{collections::BTreeMap, num::NonZeroUsize, sync::Arc, time::Duration};
use std::{num::NonZeroUsize, sync::Arc, time::Duration};

use async_trait::async_trait;
use futures::{lock::Mutex, FutureExt as _};
Expand Down Expand Up @@ -70,8 +70,7 @@ impl chain_listener::ClientContext for ClientContext {
chain.block_hash,
chain.timestamp,
chain.next_block_height,
chain.pending_block.clone(),
chain.pending_blobs.clone(),
chain.pending_proposal.clone(),
))
}

Expand All @@ -88,8 +87,7 @@ impl chain_listener::ClientContext for ClientContext {
block_hash: None,
timestamp,
next_block_height: BlockHeight::ZERO,
pending_block: None,
pending_blobs: BTreeMap::new(),
pending_proposal: None,
});
}

Expand Down
56 changes: 35 additions & 21 deletions linera-client/src/unit_tests/wallet.rs
Original file line number Diff line number Diff line change
@@ -1,31 +1,36 @@
// Copyright (c) Zefchain Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

use std::collections::BTreeMap;

use anyhow::anyhow;
use linera_base::{
crypto::KeyPair,
data_types::{Amount, Blob},
identifiers::ChainId,
data_types::{Amount, Blob, BlockHeight},
identifiers::{ChainDescription, ChainId},
};
use linera_chain::data_types::ProposedBlock;
use linera_core::{
client::PendingProposal,
test_utils::{MemoryStorageBuilder, StorageBuilder, TestBuilder},
};
use linera_core::test_utils::{MemoryStorageBuilder, StorageBuilder, TestBuilder};
use rand::SeedableRng as _;
use linera_execution::committee::Epoch;
use rand::{rngs::StdRng, SeedableRng as _};

use super::util::make_genesis_config;
use crate::{client_context::ClientContext, config::WalletState, wallet::Wallet};
use crate::{
client_context::ClientContext,
config::WalletState,
wallet::{UserChain, Wallet},
};

/// Tests whether we can correctly save a wallet that contains pending blobs.
#[test_log::test(tokio::test)]
async fn test_save_wallet_with_pending_blobs() -> anyhow::Result<()> {
let mut rng = rand::rngs::StdRng::seed_from_u64(42);
let mut rng = StdRng::seed_from_u64(42);
let storage_builder = MemoryStorageBuilder::default();
let clock = storage_builder.clock().clone();
let mut builder = TestBuilder::new(storage_builder, 4, 1).await?;
let chain_id = ChainId::root(0);
builder.add_root_chain(0, Amount::ONE).await?;
let storage = builder.make_storage().await?;
let key_pair = KeyPair::generate_from(&mut rng);

let genesis_config = make_genesis_config(&builder);

Expand All @@ -41,20 +46,29 @@ async fn test_save_wallet_with_pending_blobs() -> anyhow::Result<()> {
if wallet_path.exists() {
return Err(anyhow!("Wallet already exists!"));
}
let wallet =
let mut wallet =
WalletState::create_from_file(&wallet_path, Wallet::new(genesis_config, Some(37)))?;
let mut context = ClientContext::new_test_client_context(storage, wallet);
let blob = Blob::new_data(b"blob".to_vec());
let mut pending_blobs = BTreeMap::new();
pending_blobs.insert(blob.id(), blob);
context
.update_wallet_for_new_chain_with_pending_blobs(
chain_id,
Some(key_pair),
wallet
.add_chains(Some(UserChain::make_initial(
&mut rng,
ChainDescription::Root(0),
clock.current_time(),
pending_blobs,
)
)))
.await?;
wallet.chains_mut().next().unwrap().pending_proposal = Some(PendingProposal {
block: ProposedBlock {
chain_id,
epoch: Epoch::ZERO,
incoming_bundles: vec![],
operations: vec![],
height: BlockHeight::ZERO,
timestamp: clock.current_time(),
authenticated_signer: None,
previous_block_hash: None,
},
blobs: vec![Blob::new_data(b"blob".to_vec())],
});
let mut context = ClientContext::new_test_client_context(storage, wallet);
context.save_wallet().await?;
Ok(())
}
25 changes: 11 additions & 14 deletions linera-client/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ use std::{

use linera_base::{
crypto::{CryptoHash, CryptoRng, KeyPair},
data_types::{Blob, BlockHeight, Timestamp},
data_types::{BlockHeight, Timestamp},
ensure,
identifiers::{BlobId, ChainDescription, ChainId, Owner},
identifiers::{ChainDescription, ChainId, Owner},
};
use linera_core::{
client::{ChainClient, PendingProposal},
node::ValidatorNodeProvider,
};
use linera_chain::data_types::ProposedBlock;
use linera_core::{client::ChainClient, node::ValidatorNodeProvider};
use linera_storage::Storage;
use rand::Rng as _;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -147,8 +149,7 @@ impl Wallet {
block_hash: None,
timestamp,
next_block_height: BlockHeight(0),
pending_block: None,
pending_blobs: BTreeMap::new(),
pending_proposal: None,
};
self.insert(user_chain);
Ok(())
Expand Down Expand Up @@ -178,8 +179,7 @@ impl Wallet {
block_hash: state.block_hash(),
next_block_height: state.next_block_height(),
timestamp: state.timestamp(),
pending_block: state.pending_proposal().clone(),
pending_blobs: state.pending_blobs().clone(),
pending_proposal: state.pending_proposal().clone(),
},
);
}
Expand Down Expand Up @@ -210,8 +210,7 @@ pub struct UserChain {
pub block_hash: Option<CryptoHash>,
pub timestamp: Timestamp,
pub next_block_height: BlockHeight,
pub pending_block: Option<ProposedBlock>,
pub pending_blobs: BTreeMap<BlobId, Blob>,
pub pending_proposal: Option<PendingProposal>,
}

impl UserChain {
Expand All @@ -228,8 +227,7 @@ impl UserChain {
block_hash: None,
timestamp,
next_block_height: BlockHeight::ZERO,
pending_block: None,
pending_blobs: BTreeMap::new(),
pending_proposal: None,
}
}

Expand All @@ -242,8 +240,7 @@ impl UserChain {
block_hash: None,
timestamp,
next_block_height: BlockHeight::ZERO,
pending_block: None,
pending_blobs: BTreeMap::new(),
pending_proposal: None,
}
}
}
Loading

0 comments on commit 1fc0922

Please sign in to comment.