Skip to content

Commit

Permalink
fix: grandpa justification verify due to noteStalled operation (#385)
Browse files Browse the repository at this point in the history
  • Loading branch information
0xbillw authored Aug 2, 2024
1 parent 60c55dd commit 05fb56f
Show file tree
Hide file tree
Showing 12 changed files with 131 additions and 43 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/cestory/api/proto/ceseal_rpc.proto
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ message InitRuntimeRequest {
// Attestation provider;
// @codec scale ces_types::AttestationProvider
optional bytes attestation_provider = 6;
// Has Granpa been note-stalled
bool grandpa_note_stalled = 7;
}

// Request parameters for GetRuntimeInfo.
Expand Down
2 changes: 2 additions & 0 deletions crates/cestory/api/src/proto_generated/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ impl crate::crpc::InitRuntimeRequest {
genesis_state: crate::blocks::StorageState,
operator: Option<chain::AccountId>,
attestation_provider: Option<ces_types::AttestationProvider>,
grandpa_note_stalled: bool
) -> Self {
Self {
skip_ra,
Expand All @@ -134,6 +135,7 @@ impl crate::crpc::InitRuntimeRequest {
encoded_genesis_state: genesis_state.encode(),
encoded_operator: operator.map(|x| x.encode()),
attestation_provider: attestation_provider.map(|x| x.encode()),
grandpa_note_stalled,
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion crates/cestory/src/ceseal_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ impl<Platform: pal::Platform + Serialize + DeserializeOwned> CesealApi for RpcSe
request.decode_operator().map_err(to_status)?,
request.debug_set_key.clone(),
request.decode_attestation_provider().map_err(to_status)?,
request.grandpa_note_stalled,
)?;
Ok(Response::new(result))
}
Expand Down Expand Up @@ -731,6 +732,7 @@ impl<Platform: pal::Platform + Serialize + DeserializeOwned> Ceseal<Platform> {
operator: Option<chain::AccountId>,
debug_set_key: ::core::option::Option<Vec<u8>>,
attestation_provider: ::core::option::Option<AttestationProvider>,
grandpa_note_stalled: bool,
) -> CesealResult<pb::InitRuntimeResponse> {
if self.system.is_some() {
return Err(from_display("Runtime already initialized"))
Expand Down Expand Up @@ -791,7 +793,7 @@ impl<Platform: pal::Platform + Serialize + DeserializeOwned> Ceseal<Platform> {
// Initialize bridge
let mut light_client = LightValidation::new();
let main_bridge = light_client
.initialize_bridge(genesis.block_header.clone(), genesis.authority_set, genesis.proof)
.initialize_bridge(genesis.block_header.clone(), genesis.authority_set, genesis.proof, grandpa_note_stalled)
.expect("Bridge initialize failed");

let storage_synchronizer = Synchronizer::new_solochain(light_client, main_bridge);
Expand Down
51 changes: 38 additions & 13 deletions crates/cestory/src/light_validation/justification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ pub struct GrandpaJustification<Block: BlockT> {
/// The GRANDPA justification for block finality.
pub justification: sp_consensus_grandpa::GrandpaJustification<Block::Header>,
_block: PhantomData<Block>,
grandpa_note_stalled: bool,
}

impl<Block: BlockT> From<sp_consensus_grandpa::GrandpaJustification<Block::Header>> for GrandpaJustification<Block> {
fn from(justification: sp_consensus_grandpa::GrandpaJustification<Block::Header>) -> Self {
Self { justification, _block: Default::default() }
Self { justification, _block: Default::default(), grandpa_note_stalled: false }
}
}

Expand All @@ -64,15 +65,25 @@ impl<Block: BlockT> GrandpaJustification<Block> {
finalized_target: (Block::Hash, NumberFor<Block>),
set_id: u64,
voters: &VoterSet<AuthorityId>,
grandpa_note_stalled: bool,
) -> Result<Self, ClientError>
where
NumberFor<Block>: finality_grandpa::BlockNumberOps,
{
let justification =
GrandpaJustification::<Block>::decode(&mut &*encoded).map_err(|_| ClientError::JustificationDecode)?;
let mut justification = GrandpaJustification::<Block>::decode(&mut &*encoded).map_err(|e| {
log::error!(
"decode justification error:{:?}, block:{:?}, input(len:{}):{}",
e,
finalized_target.1,
encoded.len(),
hex::encode(encoded)
);
ClientError::JustificationDecode
})?;
justification.grandpa_note_stalled = grandpa_note_stalled;

if (justification.justification.commit.target_hash, justification.justification.commit.target_number) !=
finalized_target
if (justification.justification.commit.target_hash, justification.justification.commit.target_number)
!= finalized_target
{
let msg = "invalid commit target in grandpa justification".to_string();
Err(ClientError::BadJustification(msg))
Expand All @@ -92,8 +103,15 @@ impl<Block: BlockT> GrandpaJustification<Block> {
match finality_grandpa::validate_commit(&self.justification.commit, voters, &ancestry_chain) {
Ok(ref result) if result.is_valid() => {},
_ => {
let msg = "invalid commit in grandpa justification".to_string();
return Err(ClientError::BadJustification(msg));
if self.grandpa_note_stalled {
log::trace!(
"invalid commit in grandpa justification.
this may be due to `sudo` executing `grandpa.noteStalled()`"
);
} else {
let msg = "invalid commit in grandpa justification".to_string();
return Err(ClientError::BadJustification(msg));
}
},
}

Expand Down Expand Up @@ -126,10 +144,16 @@ impl<Block: BlockT> GrandpaJustification<Block> {
set_id,
&mut buf,
) {
log::trace!(
"invalid signature for precommit in grandpa justification.
this may be due to `sudo` executing `grandpa.noteStalled()`"
);
if self.grandpa_note_stalled {
log::trace!(
"invalid signature for precommit in grandpa justification.
this may be due to `sudo` executing `grandpa.noteStalled()`"
);
} else {
return Err(ClientError::BadJustification(
"invalid signature for precommit in grandpa justification".to_string(),
));
}
}

if base_hash == signed.precommit.target_hash {
Expand All @@ -145,10 +169,11 @@ impl<Block: BlockT> GrandpaJustification<Block> {
visited_hashes.insert(hash);
}
},
_ =>
_ => {
return Err(ClientError::BadJustification(
"invalid precommit ancestry proof in grandpa justification".to_string(),
)),
))
},
}
}

Expand Down
45 changes: 20 additions & 25 deletions crates/cestory/src/light_validation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub struct LightValidation<T: Config> {
deserialize = "BlockHeader: ::serde::de::DeserializeOwned"
))]
tracked_bridges: BTreeMap<BridgeId, BridgeInfo<T>>,
grandpa_note_stalled: bool,
}

impl<T: Config> LightValidation<T>
Expand All @@ -97,14 +98,15 @@ where
{
#[allow(clippy::new_without_default)]
pub fn new() -> Self {
LightValidation { num_bridges: 0, tracked_bridges: BTreeMap::new() }
LightValidation { num_bridges: 0, tracked_bridges: BTreeMap::new(), grandpa_note_stalled: false }
}

pub fn initialize_bridge(
&mut self,
block_header: BlockHeader,
validator_set: AuthoritySet,
proof: StorageProof,
grandpa_note_stalled: bool,
) -> Result<BridgeId> {
let state_root = block_header.state_root();

Expand All @@ -117,7 +119,8 @@ where
self.tracked_bridges.insert(new_bridge_id, bridge_info);

self.num_bridges = new_bridge_id;

self.grandpa_note_stalled = grandpa_note_stalled;

Ok(new_bridge_id)
}

Expand Down Expand Up @@ -150,13 +153,23 @@ where
let voters = &bridge.current_set;
let voter_set = VoterSet::new(voters.list.clone()).unwrap();
let voter_set_id = voters.id;
verify_grandpa_proof::<<T as frame_system::Config>::Block>(
grandpa_proof,
block_hash,
block_num.into(),

// We don't really care about the justification, as long as it's valid
match GrandpaJustification::<<T as frame_system::Config>::Block>::decode_and_verify_finalizes(
&grandpa_proof,
(block_hash, block_num.into()),
voter_set_id,
&voter_set,
)?;
self.grandpa_note_stalled,
) {
Ok(_) => {},
Err(JustificationError::JustificationDecode) if self.grandpa_note_stalled => {
log::debug!("grandpa_note_stalled is true, ignore JustificationDecode error");
},
Err(e) => {
return Err(anyhow::Error::msg(e));
},
}

match self.tracked_bridges.get_mut(&bridge_id) {
Some(bridge_info) => {
Expand Down Expand Up @@ -325,24 +338,6 @@ where
Err(anyhow::Error::msg(Error::InvalidAncestryProof))
}

fn verify_grandpa_proof<B>(
justification: EncodedJustification,
hash: B::Hash,
number: NumberFor<B>,
set_id: u64,
voters: &VoterSet<AuthorityId>,
) -> Result<()>
where
B: BlockT<Hash = H256>,
NumberFor<B>: finality_grandpa::BlockNumberOps,
{
// We don't really care about the justification, as long as it's valid
let _ = GrandpaJustification::<B>::decode_and_verify_finalizes(&justification, (hash, number), set_id, voters)
.map_err(anyhow::Error::msg)?;

Ok(())
}

impl<T: Config> fmt::Debug for LightValidation<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
Expand Down
14 changes: 14 additions & 0 deletions crates/cesxt/src/chain_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,20 @@ impl ChainApi {
Ok(launched)
}

pub async fn is_grandpa_note_stalled(&self) -> Result<bool> {
let key: Vec<Value> = vec![];
let address = subxt::dynamic::storage("TeeWorker", "NoteStalled", key);
let note_stalled = self
.storage()
.at_latest()
.await?
.fetch(&address)
.await
.context("Failed to get NoteStalled")?
.is_some();
Ok(note_stalled)
}

async fn fetch<K: Encode, V: Decode>(
&self,
pallet: &str,
Expand Down
12 changes: 12 additions & 0 deletions pallets/tee-worker/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,10 @@ pub mod pallet {
#[pallet::storage]
pub type MinimumCesealVersion<T: Config> = StorageValue<_, (u32, u32, u32), ValueQuery>;

#[pallet::storage]
#[pallet::getter(fn is_note_stalled)]
pub type NoteStalled<T: Config> = StorageValue<_, bool>;

const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

#[pallet::pallet]
Expand Down Expand Up @@ -760,6 +764,14 @@ pub mod pallet {

Ok(())
}

#[pallet::call_index(118)]
#[pallet::weight({0})]
pub fn set_note_stalled(origin: OriginFor<T>, note_stalled: bool) -> DispatchResult {
ensure_root(origin)?;
NoteStalled::<T>::put(note_stalled);
Ok(())
}
}

impl<T: Config> ces_pallet_mq::MasterPubkeySupplier for Pallet<T> {
Expand Down
36 changes: 35 additions & 1 deletion standalone/teeworker/ceseal/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion standalone/teeworker/ceseal/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
edition = "2021"
name = "ceseal"
version = "0.3.0"
version = "0.3.1"
build = "build.rs"

[profile.release]
Expand Down
2 changes: 1 addition & 1 deletion standalone/teeworker/cifrost/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cifrost"
version = "0.2.0"
version = "0.2.1"
authors = ["CESS Network"]
edition = "2021"

Expand Down
2 changes: 2 additions & 0 deletions standalone/teeworker/cifrost/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ async fn init_runtime(
info!("Inject key {}", DEV_KEY);
debug_set_key = Some(hex::decode(DEV_KEY).expect("Invalid dev key"));
}
let grandpa_note_stalled = chain_api.is_grandpa_note_stalled().await?;

let resp = ceseal_client
.init_runtime(crpc::InitRuntimeRequest::new(
Expand All @@ -142,6 +143,7 @@ async fn init_runtime(
genesis_state,
operator,
attestation_provider,
grandpa_note_stalled,
))
.await?;
Ok(resp.into_inner())
Expand Down

0 comments on commit 05fb56f

Please sign in to comment.