Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[backport] grandpa: handle error from SelectChain::finality_target #108

Merged
merged 2 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions client/consensus/grandpa/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1209,14 +1209,20 @@ where
.header(target_hash)?
.expect("Header known to exist after `finality_target` call; qed"),
Err(err) => {
warn!(
debug!(
target: LOG_TARGET,
"Encountered error finding best chain containing {:?}: couldn't find target block: {}",
block,
err,
);

return Ok(None)
// NOTE: in case the given `SelectChain` doesn't provide any block we fallback to using
// the given base block provided by the GRANDPA voter.
//
// For example, `LongestChain` will error if the given block to use as base isn't part
// of the best chain (as defined by `LongestChain`), which could happen if there was a
// re-org.
base_header.clone()
},
};

Expand Down
110 changes: 110 additions & 0 deletions client/consensus/grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1738,6 +1738,116 @@ async fn grandpa_environment_checks_if_best_block_is_descendent_of_finality_targ
);
}

// This is a regression test for an issue that was triggered by a reorg
// - https://github.com/paritytech/polkadot-sdk/issues/3487
// - https://github.com/humanode-network/humanode/issues/1104
#[tokio::test]
async fn grandpa_environment_uses_round_base_block_for_voting_if_finality_target_errors() {
use finality_grandpa::voter::Environment;
use sp_consensus::SelectChain;

let peers = &[Ed25519Keyring::Alice];
let voters = make_ids(peers);

let mut net = GrandpaTestNet::new(TestApi::new(voters), 1, 0);
let peer = net.peer(0);
let network_service = peer.network_service().clone();
let sync_service = peer.sync_service().clone();
let notification_service =
peer.take_notification_service(&grandpa_protocol_name::NAME.into()).unwrap();
let link = peer.data.lock().take().unwrap();
let client = peer.client().as_client().clone();
let select_chain = sc_consensus::LongestChain::new(peer.client().as_backend());

// create a chain that is 10 blocks long
peer.push_blocks(10, false);

let env = test_environment_with_select_chain(
&link,
None,
network_service.clone(),
sync_service,
notification_service,
select_chain.clone(),
VotingRulesBuilder::default().build(),
);

let hashof7 = client.expect_block_hash_from_id(&BlockId::Number(7)).unwrap();
let hashof8_a = client.expect_block_hash_from_id(&BlockId::Number(8)).unwrap();

// finalize the 7th block
peer.client().finalize_block(hashof7, None, false).unwrap();

assert_eq!(peer.client().info().finalized_hash, hashof7);

// simulate completed grandpa round
env.completed(
1,
finality_grandpa::round::State {
prevote_ghost: Some((hashof8_a, 8)),
finalized: Some((hashof7, 7)),
estimate: Some((hashof8_a, 8)),
completable: true,
},
Default::default(),
&finality_grandpa::HistoricalVotes::new(),
)
.unwrap();

// check simulated last completed round
assert_eq!(
env.voter_set_state.read().last_completed_round().state,
finality_grandpa::round::State {
prevote_ghost: Some((hashof8_a, 8)),
finalized: Some((hashof7, 7)),
estimate: Some((hashof8_a, 8)),
completable: true
}
);

// `hashof8_a` should be finalized next, `best_chain_containing` should return `hashof8_a`
assert_eq!(env.best_chain_containing(hashof8_a).await.unwrap().unwrap().0, hashof8_a);

// simulate reorg on block 8 by creating a fork starting at block 7 that is 10 blocks long
peer.generate_blocks_at(
BlockId::Number(7),
10,
BlockOrigin::File,
|mut builder| {
builder.push_deposit_log_digest_item(DigestItem::Other(vec![1])).unwrap();
builder.build().unwrap().block
},
false,
false,
true,
ForkChoiceStrategy::LongestChain,
);

// check that new best chain is on longest chain
assert_eq!(env.select_chain.best_chain().await.unwrap().number, 17);

// verify that last completed round has `prevote_ghost` and `estimate` blocks related to
// `hashof8_a`
assert_eq!(
env.voter_set_state.read().last_completed_round().state,
finality_grandpa::round::State {
prevote_ghost: Some((hashof8_a, 8)),
finalized: Some((hashof7, 7)),
estimate: Some((hashof8_a, 8)),
completable: true
}
);

// `hashof8_a` should be finalized next, `best_chain_containing` should still return `hashof8_a`
assert_eq!(env.best_chain_containing(hashof8_a).await.unwrap().unwrap().0, hashof8_a);

// simulate finalization of the `hashof8_a` block
peer.client().finalize_block(hashof8_a, None, false).unwrap();

// check that best chain is reorged back
assert_eq!(env.select_chain.best_chain().await.unwrap().number, 10);
}

#[tokio::test]
async fn grandpa_environment_never_overwrites_round_voter_state() {
use finality_grandpa::voter::Environment;
Expand Down