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

Zombienet tests - disputes on finalized blocks #2184

Merged
merged 30 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4ef3c22
simple passthrough interceptor
Overkillus Nov 6, 2023
46f6fc3
fixing docs
Overkillus Nov 6, 2023
ac283de
simple network for testing logs
Overkillus Nov 6, 2023
b2972c8
ancestor hash finder
Overkillus Nov 6, 2023
0eee925
remove chain spec command
Overkillus Nov 7, 2023
9d51a37
fetch session, token candidate and start a dispute
Overkillus Nov 7, 2023
51f5819
dispute fresh finalizations
Overkillus Nov 15, 2023
6276028
dispute stale finalizations
Overkillus Nov 15, 2023
10b0a63
variant unit tests
Overkillus Nov 15, 2023
6668931
better test naming
Overkillus Nov 21, 2023
2613a60
spelling
Overkillus Nov 21, 2023
c42bfa1
var rename
Overkillus Nov 21, 2023
8a4e6c6
value assert in arg test
Overkillus Nov 21, 2023
49b4601
fmt
Overkillus Nov 21, 2023
fa29314
Merge branch 'master' into mkz-ksm-inc-zombienet-tests
Overkillus Nov 21, 2023
667deba
fmt2
Overkillus Nov 21, 2023
ad70421
add tests to CI
Overkillus Nov 21, 2023
e1529f9
fix CI test name
Overkillus Nov 22, 2023
b006535
log changes
Overkillus Nov 23, 2023
b2196ab
spacing
Overkillus Nov 23, 2023
107206a
Merge branch 'master' into mkz-ksm-inc-zombienet-tests
Overkillus Nov 23, 2023
c1ba06c
test name conflict resolved
Overkillus Nov 23, 2023
a8a59a7
Merge branch 'master' into mkz-ksm-inc-zombienet-tests
Overkillus Nov 23, 2023
6143cc4
Merge branch 'master' into mkz-ksm-inc-zombienet-tests
Overkillus Nov 23, 2023
9e054df
lowering para number and network size
Overkillus Nov 24, 2023
2a8b712
size fix
Overkillus Nov 24, 2023
7c950eb
network size clarification
Overkillus Nov 24, 2023
205c48d
higher timeout in tests
Overkillus Nov 24, 2023
7cca541
switching to 1 para network
Overkillus Nov 24, 2023
7eda451
spawn_blocking -> spawn
Overkillus Nov 24, 2023
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
16 changes: 16 additions & 0 deletions .gitlab/pipeline/zombienet/polkadot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,22 @@ zombienet-polkadot-functional-0006-parachains-max-tranche0:
--local-dir="${LOCAL_DIR}/functional"
--test="0006-parachains-max-tranche0.zndsl"

zombienet-polkadot-functional-0007-dispute-freshly-finalized:
extends:
- .zombienet-polkadot-common
script:
- /home/nonroot/zombie-net/scripts/ci/run-test-local-env-manager.sh
--local-dir="${LOCAL_DIR}/functional"
--test="0007-dispute-freshly-finalized.zndsl"

zombienet-polkadot-functional-0008-dispute-old-finalized:
extends:
- .zombienet-polkadot-common
script:
- /home/nonroot/zombie-net/scripts/ci/run-test-local-env-manager.sh
--local-dir="${LOCAL_DIR}/functional"
--test="0007-dispute-old-finalized.zndsl"
Overkillus marked this conversation as resolved.
Show resolved Hide resolved

zombienet-polkadot-smoke-0001-parachains-smoke-test:
extends:
- .zombienet-polkadot-common
Expand Down
46 changes: 46 additions & 0 deletions polkadot/node/malus/src/malus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ enum NemesisVariant {
BackGarbageCandidate(BackGarbageCandidateOptions),
/// Delayed disputing of ancestors that are perfectly fine.
DisputeAncestor(DisputeAncestorOptions),
/// Delayed disputing of finalized candidates.
DisputeFinalizedCandidates(DisputeFinalizedCandidatesOptions),
}

#[derive(Debug, Parser)]
Expand Down Expand Up @@ -80,6 +82,15 @@ impl MalusCli {
finality_delay,
)?
},
NemesisVariant::DisputeFinalizedCandidates(opts) => {
let DisputeFinalizedCandidatesOptions { dispute_offset, cli } = opts;

polkadot_cli::run_node(
cli,
DisputeFinalizedCandidates { dispute_offset },
finality_delay,
)?
},
}
Ok(())
}
Expand Down Expand Up @@ -184,4 +195,39 @@ mod tests {
assert!(run.cli.run.base.bob);
});
}

#[test]
fn dispute_finalized_candidates_works() {
let cli = MalusCli::try_parse_from(IntoIterator::into_iter([
"malus",
"dispute-finalized-candidates",
"--bob",
]))
.unwrap();
assert_matches::assert_matches!(cli, MalusCli {
variant: NemesisVariant::DisputeFinalizedCandidates(run),
..
} => {
assert!(run.cli.run.base.bob);
});
}

#[test]
fn dispute_finalized_offset_value_works() {
let cli = MalusCli::try_parse_from(IntoIterator::into_iter([
"malus",
"dispute-finalized-candidates",
"--dispute-offset",
"13",
"--bob",
]))
.unwrap();
assert_matches::assert_matches!(cli, MalusCli {
variant: NemesisVariant::DisputeFinalizedCandidates(opts),
..
} => {
assert_eq!(opts.dispute_offset, 13); // This line checks that dispute_offset is correctly set to 13
assert!(opts.cli.run.base.bob);
});
}
}
2 changes: 1 addition & 1 deletion polkadot/node/malus/src/variants/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! Implements common code for nemesis. Currently, only `FakeValidationResult`
//! Implements common code for nemesis. Currently, only `ReplaceValidationResult`
//! interceptor is implemented.
use crate::{
Overkillus marked this conversation as resolved.
Show resolved Hide resolved
interceptor::*,
Expand Down
260 changes: 260 additions & 0 deletions polkadot/node/malus/src/variants/dispute_finalized_candidates.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
// Copyright (C) Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! A malicious node variant that attempts to dispute finalized candidates.
//!
//! This malus variant behaves honestly in backing and approval voting.
//! The maliciousness comes from emitting an extra dispute statement on top of the other ones.
//!
//! Some extra quirks which generally should be insignificant:
//! - The malus node will not dispute at session boundaries
//! - The malus node will not dispute blocks it backed itself
//!
//! Attention: For usage with `zombienet` only!

#![allow(missing_docs)]

use futures::channel::oneshot;
use polkadot_cli::{
prepared_overseer_builder,
service::{
AuthorityDiscoveryApi, AuxStore, BabeApi, Block, Error, HeaderBackend, Overseer,
OverseerConnector, OverseerGen, OverseerGenArgs, OverseerHandle, ParachainHost,
ProvideRuntimeApi,
},
Cli,
};
use polkadot_node_subsystem::{messages::ApprovalVotingMessage, SpawnGlue};
use polkadot_node_subsystem_types::{DefaultSubsystemClient, OverseerSignal};
use polkadot_node_subsystem_util::request_candidate_events;
use polkadot_primitives::CandidateEvent;
use sp_core::traits::SpawnNamed;

// Filter wrapping related types.
use crate::{interceptor::*, shared::MALUS};

use std::sync::Arc;

/// Wraps around ApprovalVotingSubsystem and replaces it.
/// Listens to finalization messages and if possible triggers disputes for their ancestors.
#[derive(Clone)]
struct AncestorDisputer<Spawner> {
spawner: Spawner, //stores the actual ApprovalVotingSubsystem spawner
dispute_offset: u32, /* relative depth of the disputed block to the finalized block,
* 0=finalized, 1=parent of finalized etc */
}

impl<Sender, Spawner> MessageInterceptor<Sender> for AncestorDisputer<Spawner>
where
Sender: overseer::ApprovalVotingSenderTrait + Clone + Send + 'static,
Spawner: overseer::gen::Spawner + Clone + 'static,
{
type Message = ApprovalVotingMessage;

/// Intercept incoming `OverseerSignal::BlockFinalized' and pass the rest as normal.
fn intercept_incoming(
&self,
subsystem_sender: &mut Sender,
msg: FromOrchestra<Self::Message>,
) -> Option<FromOrchestra<Self::Message>> {
match msg {
FromOrchestra::Communication { msg } => Some(FromOrchestra::Communication { msg }),
FromOrchestra::Signal(OverseerSignal::BlockFinalized(
finalized_hash,
finalized_height,
)) => {
gum::info!(
target: MALUS,
"😈 Block Finalization Interception! Block: {:?}", finalized_hash,
);

//Ensure that the chain is long enough for the target ancestor to exist
if finalized_height <= self.dispute_offset {
return Some(FromOrchestra::Signal(OverseerSignal::BlockFinalized(
finalized_hash,
finalized_height,
)))
}

let dispute_offset = self.dispute_offset;
let mut sender = subsystem_sender.clone();
self.spawner.spawn_blocking(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jFYI, spawn_blocking means that this future will get its own thread (as opposed to being shared on a threadpool), not that this call will block until fully executed. we usually use that for CPU heavy futures, which is not the case here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I saw at least one other zombienet test that did spawn_blocking, can you check if it's needed there? I guess probably not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zombienet test

I mean malus variant

Copy link
Contributor Author

@Overkillus Overkillus Nov 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I saw at least one other zombienet test that did spawn_blocking

In fact every malus variant use spawn_blocking as it's the implementation of the shared interceptor logic (common.rs) as well as the 2 custom interceptors (the new one I added here and suggest_garbage_candidate). None of them seem super heavy as the common is simply faking validation and dumps precomputed results.

@sandreim do we wanna keep it as the spawn_blocking? Why was it blocking in the first place? example in common.rs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we don't need spawn_blocking in any place in malus. Let's switch to spawn

"malus-dispute-finalized-block",
Some("malus"),
Box::pin(async move {
// Query chain for the block hash at the target depth
let (tx, rx) = oneshot::channel();
sender
.send_message(ChainApiMessage::FinalizedBlockHash(
finalized_height - dispute_offset,
tx,
))
.await;
let disputable_hash = match rx.await {
Ok(Ok(Some(hash))) => {
gum::info!(
Overkillus marked this conversation as resolved.
Show resolved Hide resolved
target: MALUS,
"😈 Time to search {:?}`th ancestor! Block: {:?}", dispute_offset, hash,
);
hash
},
_ => {
gum::info!(
target: MALUS,
"😈 Seems the target is not yet finalized! Nothing to dispute."
);
return // Early return from the async block
},
};

// Fetch all candidate events for the target ancestor
let events =
request_candidate_events(disputable_hash, &mut sender).await.await;
let events = match events {
Ok(Ok(events)) => events,
Ok(Err(e)) => {
gum::error!(
target: MALUS,
"😈 Failed to fetch candidate events: {:?}", e
);
return // Early return from the async block
},
Err(e) => {
gum::error!(
target: MALUS,
"😈 Failed to fetch candidate events: {:?}", e
);
return // Early return from the async block
},
};

// Extract a token candidate from the events to use for disputing
let event = events.iter().find(|event| {
matches!(event, CandidateEvent::CandidateIncluded(_, _, _, _))
});
let candidate = match event {
Some(CandidateEvent::CandidateIncluded(candidate, _, _, _)) =>
candidate,
_ => {
gum::error!(
target: MALUS,
"😈 No candidate included event found! Nothing to dispute."
);
return // Early return from the async block
},
};

// Extract the candidate hash from the candidate
let candidate_hash = candidate.hash();

// Fetch the session index for the candidate
let (tx, rx) = oneshot::channel();
sender
.send_message(RuntimeApiMessage::Request(
disputable_hash,
RuntimeApiRequest::SessionIndexForChild(tx),
))
.await;
let session_index = match rx.await {
Ok(Ok(session_index)) => session_index,
_ => {
gum::error!(
target: MALUS,
"😈 Failed to fetch session index for candidate."
);
return // Early return from the async block
},
};
gum::info!(
target: MALUS,
"😈 Disputing candidate with hash: {:?} in session {:?}", candidate_hash, session_index,
);

// Start dispute
sender.send_unbounded_message(
DisputeCoordinatorMessage::IssueLocalStatement(
session_index,
candidate_hash,
candidate.clone(),
false, // indicates candidate is invalid -> dispute starts
),
);
}),
);

// Passthrough the finalization signal as usual (using it as hook only)
Some(FromOrchestra::Signal(OverseerSignal::BlockFinalized(
finalized_hash,
finalized_height,
)))
},
FromOrchestra::Signal(signal) => Some(FromOrchestra::Signal(signal)),
}
}
}

//----------------------------------------------------------------------------------

#[derive(Debug, clap::Parser)]
#[clap(rename_all = "kebab-case")]
#[allow(missing_docs)]
pub struct DisputeFinalizedCandidatesOptions {
/// relative depth of the disputed block to the finalized block, 0=finalized, 1=parent of
/// finalized etc
#[clap(long, ignore_case = true, default_value_t = 2, value_parser = clap::value_parser!(u32).range(0..=50))]
pub dispute_offset: u32,

#[clap(flatten)]
pub cli: Cli,
}
Overkillus marked this conversation as resolved.
Show resolved Hide resolved
/// DisputeFinalizedCandidates implementation wrapper which implements `OverseerGen` glue.
pub(crate) struct DisputeFinalizedCandidates {
/// relative depth of the disputed block to the finalized block, 0=finalized, 1=parent of
/// finalized etc
pub dispute_offset: u32,
}

impl OverseerGen for DisputeFinalizedCandidates {
fn generate<Spawner, RuntimeClient>(
&self,
connector: OverseerConnector,
args: OverseerGenArgs<'_, Spawner, RuntimeClient>,
) -> Result<
(Overseer<SpawnGlue<Spawner>, Arc<DefaultSubsystemClient<RuntimeClient>>>, OverseerHandle),
Error,
>
where
RuntimeClient: 'static + ProvideRuntimeApi<Block> + HeaderBackend<Block> + AuxStore,
RuntimeClient::Api: ParachainHost<Block> + BabeApi<Block> + AuthorityDiscoveryApi<Block>,
Spawner: 'static + SpawnNamed + Clone + Unpin,
{
gum::info!(
target: MALUS,
"😈 Started Malus node that disputes finalized blocks after they are {:?} finalizations deep.",
&self.dispute_offset,
);

let ancestor_disputer = AncestorDisputer {
spawner: SpawnGlue(args.spawner.clone()),
dispute_offset: self.dispute_offset,
};

prepared_overseer_builder(args)?
.replace_approval_voting(move |cb| InterceptedSubsystem::new(cb, ancestor_disputer))
.build_with_connector(connector)
.map_err(|e| e.into())
}
}
2 changes: 2 additions & 0 deletions polkadot/node/malus/src/variants/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@

mod back_garbage_candidate;
mod common;
mod dispute_finalized_candidates;
mod dispute_valid_candidates;
mod suggest_garbage_candidate;

pub(crate) use self::{
back_garbage_candidate::{BackGarbageCandidateOptions, BackGarbageCandidates},
dispute_finalized_candidates::{DisputeFinalizedCandidates, DisputeFinalizedCandidatesOptions},
dispute_valid_candidates::{DisputeAncestorOptions, DisputeValidCandidates},
suggest_garbage_candidate::{SuggestGarbageCandidateOptions, SuggestGarbageCandidates},
};
Expand Down
Loading