From a4f483052b436a62eb5fbc6fc2cf0cd6e81dc8e9 Mon Sep 17 00:00:00 2001 From: Chris Czub Date: Wed, 21 Aug 2024 20:27:33 -0400 Subject: [PATCH] Cleanup --- crates/cnidarium/src/store/substore.rs | 7 - crates/core/app/src/app/mod.rs | 1 - crates/core/app/src/server/consensus.rs | 16 -- crates/core/app/tests/common/ibc_tests/mod.rs | 158 ------------------ .../core/app/tests/common/ibc_tests/node.rs | 19 +-- .../app/tests/mock_consensus_block_proving.rs | 98 ++++++----- .../mock-consensus/src/builder/init_chain.rs | 2 +- 7 files changed, 62 insertions(+), 239 deletions(-) diff --git a/crates/cnidarium/src/store/substore.rs b/crates/cnidarium/src/store/substore.rs index 6234780f46..876a94b744 100644 --- a/crates/cnidarium/src/store/substore.rs +++ b/crates/cnidarium/src/store/substore.rs @@ -229,13 +229,6 @@ impl SubstoreSnapshot { ) -> Result<(Option>, ics23::CommitmentProof)> { let version = self.version(); let tree = jmt::Sha256Jmt::new(self); - let rh = tree.get_root_hash(version)?; - println!( - "HERE in SubstoreSnapshot get_with_proof, key: {}, version: {}, root hash: {}", - hex::encode(key.clone()), - version, - hex::encode(rh.0) - ); tree.get_with_ics23_proof(key, version) } diff --git a/crates/core/app/src/app/mod.rs b/crates/core/app/src/app/mod.rs index 967e949813..cec052d4ea 100644 --- a/crates/core/app/src/app/mod.rs +++ b/crates/core/app/src/app/mod.rs @@ -628,7 +628,6 @@ impl App { } tracing::debug!(?jmt_root, "finished committing state"); - println!("finished committing state {}", hex::encode(jmt_root)); // Get the latest version of the state, now that we've committed it. self.state = Arc::new(StateDelta::new(storage.latest_snapshot())); diff --git a/crates/core/app/src/server/consensus.rs b/crates/core/app/src/server/consensus.rs index b530c78e0e..b60e1aaffe 100644 --- a/crates/core/app/src/server/consensus.rs +++ b/crates/core/app/src/server/consensus.rs @@ -242,25 +242,9 @@ impl Consensus { } async fn commit(&mut self) -> Result { - let storage_revision_height = self.storage.latest_snapshot().version(); - let storage_root = self.storage.latest_snapshot().root_hash().await?; - println!( - "BEFORE commit storage height is {} and storage root is {}", - storage_revision_height, - hex::encode(storage_root.0) - ); - let app_hash = self.app.commit(self.storage.clone()).await; tracing::info!(?app_hash, "committed block"); - let storage_revision_height = self.storage.latest_snapshot().version(); - let storage_root = self.storage.latest_snapshot().root_hash().await?; - println!( - "AFTER commit storage height is {} and storage root is {}", - storage_revision_height, - hex::encode(storage_root.0) - ); - Ok(response::Commit { data: app_hash.0.to_vec().into(), retain_height: 0u32.into(), diff --git a/crates/core/app/tests/common/ibc_tests/mod.rs b/crates/core/app/tests/common/ibc_tests/mod.rs index 7f275a659f..4c03e8a44c 100644 --- a/crates/core/app/tests/common/ibc_tests/mod.rs +++ b/crates/core/app/tests/common/ibc_tests/mod.rs @@ -191,161 +191,3 @@ pub fn get_verified_genesis() -> Result { Ok(genesis) } - -// export keys and genesis to files for testing -// TODO: this is kind of useful to have in a test utility crate -// pub fn export_key() { -// // Write this node's priv_validator_key.json -// let priv_validator_key_filepath = "/tmp/priv_validator_key.json"; -// tracing::debug!(priv_validator_key_filepath = %priv_validator_key_filepath, "writing validator private key"); -// let mut priv_validator_key_file = File::create(priv_validator_key_filepath)?; -// // let priv_validator_key: PrivValidatorKey = sk_b.into(); -// let pub_key_a = -// tendermint::PublicKey::from_raw_ed25519(vk_a.as_bytes()).expect("pub key present"); -// let pub_key_b = -// tendermint::PublicKey::from_raw_ed25519(vk_b.as_bytes()).expect("pub key present"); -// let address_a = tendermint::account::Id::new( -// ::digest(vk_a.as_bytes()).as_slice()[0..20] -// .try_into() -// .expect(""), -// ); -// let priv_validator_key = PrivValidatorKey { -// address: address_a, -// pub_key: pub_key_a, -// priv_key: tendermint::PrivateKey::Ed25519( -// tendermint::crypto::ed25519::SigningKey::try_from(sk_a.clone().as_bytes())?, -// ), -// }; -// priv_validator_key_file -// .write_all(serde_json::to_string_pretty(&priv_validator_key)?.as_bytes())?; -// // TODO: make it possible to flag exporting the app state, keys, etc. -// // to files possible on the builder -// { -// // genesis contents need to contain validator information in the app state -// let mut genesis_contents = -// genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()); - -// let spend_key_a = SpendKey::from(vkeys_a.validator_spend_key.clone()); -// let spend_key_b = SpendKey::from(vkeys_b.validator_spend_key.clone()); -// tendermint::PublicKey::from_raw_ed25519(&vkeys_a.validator_cons_pk.to_bytes()) -// .ok_or_else(|| anyhow::anyhow!("invalid ed25519 consensus pubkey")) -// .unwrap(); -// let validator_a = Validator { -// identity_key: Some(IdentityKey(vkeys_a.validator_id_vk.into()).into()), -// governance_key: Some(GovernanceKey(spend_key_a.spend_auth_key().into()).into()), -// consensus_key: vkeys_a.validator_cons_pk.to_bytes(), -// name: "test".to_string(), -// website: "https://example.com".to_string(), -// description: "test".to_string(), -// enabled: true, -// funding_streams: vec![], -// sequence_number: 0, -// }; -// let validator_b = Validator { -// identity_key: Some(IdentityKey(vkeys_b.validator_id_vk.into()).into()), -// governance_key: Some(GovernanceKey(spend_key_b.spend_auth_key().into()).into()), -// consensus_key: vkeys_b.validator_cons_pk.to_bytes(), -// name: "test".to_string(), -// website: "https://example.com".to_string(), -// description: "test".to_string(), -// enabled: true, -// funding_streams: vec![], -// sequence_number: 0, -// }; -// genesis_contents -// .stake_content -// .validators -// .push(validator_a.clone()); -// // TODO: actually let's only do one validator per chain for now -// // since it's easier to validate against cometbft -// // genesis_contents -// // .stake_content -// // .validators -// // .push(validator_b.clone()); - -// // the two validators need some initial delegations - -// let identity_key_a: IdentityKey = IdentityKey( -// spend_key_a -// .full_viewing_key() -// .spend_verification_key() -// .clone() -// .into(), -// ); -// let delegation_id_a = DelegationToken::from(&identity_key_a).denom(); -// let ivk_a = spend_key_a.incoming_viewing_key(); -// genesis_contents -// .shielded_pool_content -// .allocations -// .push(Allocation { -// address: ivk_a.payment_address(0u32.into()).0, -// raw_amount: (25_000 * 10u128.pow(6)).into(), -// raw_denom: delegation_id_a.to_string(), -// }); - -// let identity_key_b: IdentityKey = IdentityKey( -// spend_key_b -// .full_viewing_key() -// .spend_verification_key() -// .clone() -// .into(), -// ); -// let delegation_id_b = DelegationToken::from(&identity_key_b).denom(); -// let ivk_b = spend_key_b.incoming_viewing_key(); -// // genesis_contents -// // .shielded_pool_content -// // .allocations -// // .push(Allocation { -// // address: ivk_b.payment_address(0u32.into()).0, -// // raw_amount: (25_000 * 10u128.pow(6)).into(), -// // raw_denom: delegation_id_b.to_string(), -// // }); - -// let genesis = Genesis { -// genesis_time: start_time.clone(), -// chain_id: genesis_contents -// .chain_id -// .parse::() -// .context("failed to parse chain ID")?, -// initial_height: 0, -// consensus_params: tendermint::consensus::Params { -// abci: AbciParams::default(), -// block: tendermint::block::Size { -// // 1MB -// max_bytes: MAX_BLOCK_TXS_PAYLOAD_BYTES as u64, -// // Set to infinity since a chain running Penumbra won't use -// // cometbft's notion of gas. -// max_gas: -1, -// // Minimum time increment between consecutive blocks. -// time_iota_ms: 500, -// }, -// evidence: tendermint::evidence::Params { -// // We should keep this in approximate sync with the recommended default for -// // `StakeParameters::unbonding_delay`, this is roughly a week. -// max_age_num_blocks: 130000, -// // Similarly, we set the max age duration for evidence to be a little over a week. -// max_age_duration: tendermint::evidence::Duration(Duration::from_secs( -// 650000, -// )), -// // 30KB -// max_bytes: MAX_EVIDENCE_SIZE_BYTES as i64, -// }, -// validator: tendermint::consensus::params::ValidatorParams { -// pub_key_types: vec![Algorithm::Ed25519], -// }, -// version: Some(tendermint::consensus::params::VersionParams { app: 0 }), -// }, -// // always empty in genesis json -// app_hash: tendermint::AppHash::default(), -// app_state: penumbra_app::genesis::AppState::Content(genesis_contents), -// // Set empty validator set for Tendermint config, which falls back to reading -// // validators from the AppState, via ResponseInitChain: -// // https://docs.tendermint.com/v0.32/tendermint-core/using-tendermint.html -// validators: vec![], -// }; -// let genesis_file = "/tmp/genesis.json"; -// tracing::info!(?genesis_file, "writing genesis file to comet config"); -// let mut genesis_file = File::create(genesis_file)?; -// genesis_file.write_all(serde_json::to_string_pretty(&genesis)?.as_bytes())?; -// }; -// } diff --git a/crates/core/app/tests/common/ibc_tests/node.rs b/crates/core/app/tests/common/ibc_tests/node.rs index 35276b9279..bbe4edb122 100644 --- a/crates/core/app/tests/common/ibc_tests/node.rs +++ b/crates/core/app/tests/common/ibc_tests/node.rs @@ -90,17 +90,17 @@ impl TestNodeWithIBC { .tap_ok(|e| tracing::info!(hash = %e.last_app_hash_hex(), "finished init chain"))? }; - // TODO: hacky lol - let (_other_suffix, index) = match suffix { - "a" => ("b", 0), - "b" => ("a", 1), + // to select a port number just index on the suffix for now + let index = match suffix { + "a" => 0, + "b" => 1, _ => unreachable!("update this hack"), }; let grpc_url = format!("http://127.0.0.1:808{}", index) // see #4517 .parse::()? .tap(|url| tracing::debug!(%url, "parsed grpc url")); - println!("spawning gRPC..."); + tracing::info!("spawning gRPC..."); // Spawn the node's RPC server. let _rpc_server = { let make_svc = penumbra_app::rpc::router( @@ -111,14 +111,14 @@ impl TestNodeWithIBC { .into_router() .layer(tower_http::cors::CorsLayer::permissive()) .into_make_service() - .tap(|_| println!("initialized rpc service")); + .tap(|_| tracing::info!("initialized rpc service")); let [addr] = grpc_url .socket_addrs(|| None)? .try_into() .expect("grpc url can be turned into a socket address"); let server = axum_server::bind(addr).serve(make_svc); tokio::spawn(async { server.await.expect("grpc server returned an error") }) - .tap(|_| println!("grpc server is running")) + .tap(|_| tracing::info!("grpc server is running")) }; time::sleep(time::Duration::from_secs(1)).await; @@ -310,11 +310,6 @@ impl TestNodeWithIBC { revision_height: 0, }), }; - println!( - "Created header: {} with trusted height: {}", - hex::encode(header.signed_header.header.app_hash.as_bytes().to_vec()), - header.trusted_height - ); Ok(header) } } diff --git a/crates/core/app/tests/mock_consensus_block_proving.rs b/crates/core/app/tests/mock_consensus_block_proving.rs index f4be2d71d6..f522020769 100644 --- a/crates/core/app/tests/mock_consensus_block_proving.rs +++ b/crates/core/app/tests/mock_consensus_block_proving.rs @@ -247,11 +247,11 @@ async fn verify_storage_proof_simple() -> anyhow::Result<()> { assert_eq!(u64::from(latest_height), storage_revision_height); // Try fetching the client state via the IBC API - let node_last_app_hash = node.last_app_hash(); + let node_last_app_hash = node.last_app_hash().to_vec(); tracing::debug!( "making IBC client state request at height {} and hash {}", latest_height, - hex::encode(node_last_app_hash) + hex::encode(&node_last_app_hash) ); let ibc_client_state_response = ibc_client_query_client .client_state(QueryClientStateRequest { @@ -260,11 +260,51 @@ async fn verify_storage_proof_simple() -> anyhow::Result<()> { .await? .into_inner(); + assert!( + ibc_client_state_response.client_state.as_ref().is_some() + && !ibc_client_state_response + .client_state + .as_ref() + .unwrap() + .value + .is_empty() + ); + let ibc_proof = MerkleProof::decode(ibc_client_state_response.clone().proof.as_slice())?; let ibc_value = ibc_client_state_response.client_state.unwrap(); assert_eq!(ibc_value.encode_to_vec(), value); + // The current height of the node should be one behind the proof height. + assert_eq!( + u64::from(latest_height) + 1, + ibc_client_state_response + .proof_height + .clone() + .unwrap() + .revision_height + ); + + let proof_block: penumbra_proto::util::tendermint_proxy::v1::GetBlockByHeightResponse = + tendermint_proxy_service_client + .get_block_by_height(GetBlockByHeightRequest { + height: ibc_client_state_response + .proof_height + .clone() + .unwrap() + .revision_height + .try_into()?, + }) + .await? + .into_inner(); + + // The proof block should be nonexistent because we haven't finalized the in-progress + // block yet. + assert!(proof_block.block.is_none()); + + // Execute a block to finalize the proof block. + node.block().execute().await?; + // We should be able to get the block from the proof_height associated with // the proof and use the app_hash as the jmt root and succeed in proving: let proof_block: penumbra_proto::util::tendermint_proxy::v1::GetBlockByHeightResponse = @@ -289,49 +329,19 @@ async fn verify_storage_proof_simple() -> anyhow::Result<()> { .revision_height, proof_block.block.clone().unwrap().header.unwrap().height as u64 ); - // The node height when we directly retrieved the last app hash - // should match the proof height - assert_eq!( - ibc_client_state_response - .proof_height - .clone() - .unwrap() - .revision_height, - u64::from(latest_height) - ); - // TODO: these tests fail - if false { - // the proof block's app hash should match - assert_eq!( - node_last_app_hash, - proof_block.block.clone().unwrap().header.unwrap().app_hash, - "node claimed app hash for height {} was {}, however block header contained {}", - latest_height, - hex::encode(node_last_app_hash), - hex::encode(proof_block.block.clone().unwrap().header.unwrap().app_hash) - ); - println!( - "proof height: {} proof_block_root: {:?}", - ibc_client_state_response - .proof_height - .unwrap() - .revision_height, - hex::encode(proof_block.block.clone().unwrap().header.unwrap().app_hash) - ); - let proof_block_root = MerkleRoot { - hash: proof_block.block.unwrap().header.unwrap().app_hash, - }; - ibc_proof - .verify_membership( - &proof_specs, - proof_block_root, - merkle_path, - ibc_value.encode_to_vec(), - 0, - ) - .expect("the ibc proof should validate against the root of the proof_height's block"); - } + let proof_block_root = MerkleRoot { + hash: proof_block.block.unwrap().header.unwrap().app_hash, + }; + ibc_proof + .verify_membership( + &proof_specs, + proof_block_root, + merkle_path, + ibc_value.encode_to_vec(), + 0, + ) + .expect("the ibc proof should validate against the root of the proof_height's block"); Ok(()) .tap(|_| drop(node)) diff --git a/crates/test/mock-consensus/src/builder/init_chain.rs b/crates/test/mock-consensus/src/builder/init_chain.rs index beea8b39a1..909e61c074 100644 --- a/crates/test/mock-consensus/src/builder/init_chain.rs +++ b/crates/test/mock-consensus/src/builder/init_chain.rs @@ -171,9 +171,9 @@ impl Builder { timestamp, ts_callback: ts_callback.unwrap_or(Box::new(default_ts_callback)), chain_id, + consensus_params_hash: sha2::Sha256::digest(hashed_params.encode_to_vec()).to_vec(), // No last commit for the genesis block. last_commit: None, - consensus_params_hash: sha2::Sha256::digest(hashed_params.encode_to_vec()).to_vec(), }) }