Skip to content

Commit c5782df

Browse files
authored
[testnet] Sync if we learn about a new round while requesting a timeout. (#4912) (#4914)
Backport of #4912. ## Motivation If a client requests a timeout certificate and the validators return a `WrongRound` error with a _higher_ round, the client currently fails to update its local node about the new round, and thus cannot propose a block. ## Proposal Synchronize the chain state if we get a `WrongRound` error with a round higher than the one we see locally. ## Test Plan A regression test was added (by Claude). ## Release Plan - These changes should be released in a new SDK. ## Links - PR to main: #4912 - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
1 parent b565367 commit c5782df

File tree

3 files changed

+226
-39
lines changed

3 files changed

+226
-39
lines changed

linera-core/src/client/mod.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ pub struct Client<Env: Environment> {
151151
environment: Env,
152152
/// Local node to manage the execution state and the local storage of the chains that we are
153153
/// tracking.
154-
local_node: LocalNodeClient<Env::Storage>,
154+
pub local_node: LocalNodeClient<Env::Storage>,
155155
/// Manages the requests sent to validator nodes.
156156
requests_scheduler: RequestsScheduler<Env>,
157157
/// The admin chain ID.
@@ -536,7 +536,7 @@ impl<Env: Environment> Client<Env> {
536536
/// Submits a validated block for finalization and returns the confirmed block certificate.
537537
#[instrument(level = "trace", skip_all)]
538538
async fn finalize_block(
539-
&self,
539+
self: &Arc<Self>,
540540
committee: &Committee,
541541
certificate: ValidatedBlockCertificate,
542542
) -> Result<ConfirmedBlockCertificate, ChainClientError> {
@@ -557,7 +557,7 @@ impl<Env: Environment> Client<Env> {
557557
/// Submits a block proposal to the validators.
558558
#[instrument(level = "trace", skip_all)]
559559
async fn submit_block_proposal<T: ProcessableCertificate>(
560-
&self,
560+
self: &Arc<Self>,
561561
committee: &Committee,
562562
proposal: Box<BlockProposal>,
563563
value: T,
@@ -581,7 +581,7 @@ impl<Env: Environment> Client<Env> {
581581
/// Broadcasts certified blocks to validators.
582582
#[instrument(level = "trace", skip_all, fields(chain_id, block_height, delivery))]
583583
async fn communicate_chain_updates(
584-
&self,
584+
self: &Arc<Self>,
585585
committee: &Committee,
586586
chain_id: ChainId,
587587
height: BlockHeight,
@@ -595,7 +595,7 @@ impl<Env: Environment> Client<Env> {
595595
|remote_node| {
596596
let mut updater = ValidatorUpdater {
597597
remote_node,
598-
local_node: self.local_node.clone(),
598+
client: self.clone(),
599599
admin_id: self.admin_id,
600600
};
601601
Box::pin(async move {
@@ -617,7 +617,7 @@ impl<Env: Environment> Client<Env> {
617617
/// and returns a certificate.
618618
#[instrument(level = "trace", skip_all)]
619619
async fn communicate_chain_action<T: CertificateValue>(
620-
&self,
620+
self: &Arc<Self>,
621621
committee: &Committee,
622622
action: CommunicateAction,
623623
value: T,
@@ -630,7 +630,7 @@ impl<Env: Environment> Client<Env> {
630630
|remote_node| {
631631
let mut updater = ValidatorUpdater {
632632
remote_node,
633-
local_node: self.local_node.clone(),
633+
client: self.clone(),
634634
admin_id: self.admin_id,
635635
};
636636
let action = action.clone();
@@ -1062,7 +1062,7 @@ impl<Env: Environment> Client<Env> {
10621062
/// Downloads any certificates from the specified validator that we are missing for the given
10631063
/// chain, and processes them.
10641064
#[instrument(level = "trace", skip(self, remote_node, chain_id))]
1065-
async fn synchronize_chain_state_from(
1065+
pub(crate) async fn synchronize_chain_state_from(
10661066
&self,
10671067
remote_node: &RemoteNode<Env::ValidatorNode>,
10681068
chain_id: ChainId,

linera-core/src/unit_tests/client_tests.rs

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2101,6 +2101,111 @@ where
21012101
Ok(())
21022102
}
21032103

2104+
#[test_case(MemoryStorageBuilder::default(); "memory")]
2105+
#[cfg_attr(feature = "storage-service", test_case(ServiceStorageBuilder::new(); "storage_service"))]
2106+
#[cfg_attr(feature = "rocksdb", test_case(RocksDbStorageBuilder::new().await; "rocks_db"))]
2107+
#[cfg_attr(feature = "dynamodb", test_case(DynamoDbStorageBuilder::default(); "dynamo_db"))]
2108+
#[cfg_attr(feature = "scylladb", test_case(ScyllaDbStorageBuilder::default(); "scylla_db"))]
2109+
#[test_log::test(tokio::test)]
2110+
async fn test_request_leader_timeout_client_behind_validators<B>(
2111+
storage_builder: B,
2112+
) -> anyhow::Result<()>
2113+
where
2114+
B: StorageBuilder,
2115+
{
2116+
let signer = InMemorySigner::new(None);
2117+
let clock = storage_builder.clock().clone();
2118+
let mut builder = TestBuilder::new(storage_builder, 4, 1, signer).await?;
2119+
let client = builder.add_root_chain(1, Amount::from_tokens(3)).await?;
2120+
let observer = builder.add_root_chain(2, Amount::ZERO).await?;
2121+
let chain_id = client.chain_id();
2122+
let observer_id = observer.chain_id();
2123+
let owner0 = client.identity().await.unwrap();
2124+
let owner1 = AccountSecretKey::generate().public().into();
2125+
2126+
// Set up multi-owner chain.
2127+
let owners = [(owner0, 100), (owner1, 100)];
2128+
let ownership = ChainOwnership::multiple(owners, 0, TimeoutConfig::default());
2129+
client.change_ownership(ownership.clone()).await.unwrap();
2130+
2131+
// Advance to a round where owner1 is the leader (so owner0 is not).
2132+
let round_where_owner0_not_leader = loop {
2133+
let manager = client.chain_info().await.unwrap().manager;
2134+
if manager.leader == Some(owner1) {
2135+
break manager.current_round;
2136+
}
2137+
clock.set(manager.round_timeout.unwrap());
2138+
client.request_leader_timeout().await.unwrap();
2139+
};
2140+
let round_number = match round_where_owner0_not_leader {
2141+
Round::SingleLeader(n) => n,
2142+
round => panic!("Unexpected round {round:?}"),
2143+
};
2144+
2145+
// Now create a second client on the same chain to advance validators ahead
2146+
// to the next round, where owner0 will be the leader.
2147+
let client2 = builder
2148+
.make_client(chain_id, None, BlockHeight::ZERO)
2149+
.await?;
2150+
2151+
// Sync client2 to get the current state.
2152+
client2.synchronize_from_validators().await?;
2153+
2154+
// Advance validators one more round using client2.
2155+
let timeout = client2
2156+
.chain_info()
2157+
.await
2158+
.unwrap()
2159+
.manager
2160+
.round_timeout
2161+
.expect("round_timeout should be set after sync");
2162+
clock.set(timeout);
2163+
client2.request_leader_timeout().await.unwrap();
2164+
2165+
// Validators are now in round_number + 1.
2166+
let validator_round = Round::SingleLeader(round_number + 1);
2167+
builder
2168+
.check_that_validators_are_in_round(chain_id, BlockHeight::from(1), validator_round, 3)
2169+
.await;
2170+
2171+
// At this point:
2172+
// - The client's local state shows it's in round_number (where owner1 is the leader).
2173+
// - The validators are in round_number + 1 (where owner0 is the leader).
2174+
// When the client tries to transfer, it will see it's not the leader in its current round
2175+
// and will try to request a timeout. That timeout request for round_number will be rejected
2176+
// by validators (who are in round_number + 1). The client should handle this error,
2177+
// sync to the actual round, and complete the transfer.
2178+
2179+
let result = client
2180+
.transfer(
2181+
AccountOwner::CHAIN,
2182+
Amount::ONE,
2183+
Account::chain(observer_id),
2184+
)
2185+
.await;
2186+
2187+
// The transfer should succeed after the client discovers it's actually the leader in the validator's current round.
2188+
match result {
2189+
Ok(ClientOutcome::Committed(_)) => {
2190+
// Success! The client handled the round mismatch and completed the transfer.
2191+
}
2192+
Ok(ClientOutcome::WaitForTimeout(_)) => {
2193+
panic!(
2194+
"Transfer returned WaitForTimeout, but the client should have discovered \
2195+
it's the leader in the validator's current round and completed the transfer."
2196+
);
2197+
}
2198+
Err(e) => {
2199+
panic!(
2200+
"Transfer failed with error: {e:?}. The client should have handled the \
2201+
round mismatch automatically by syncing with validators."
2202+
);
2203+
}
2204+
}
2205+
2206+
Ok(())
2207+
}
2208+
21042209
#[test_case(MemoryStorageBuilder::default(); "memory")]
21052210
#[cfg_attr(feature = "storage-service", test_case(ServiceStorageBuilder::new(); "storage_service"))]
21062211
#[cfg_attr(feature = "rocksdb", test_case(RocksDbStorageBuilder::new().await; "rocks_db"))]

0 commit comments

Comments
 (0)