Skip to content

Commit

Permalink
Merge branch 'tomas/rm-end-block-empty-hash' (#3136)
Browse files Browse the repository at this point in the history
* tomas/rm-end-block-empty-hash:
  changelog: add #3136
  test/e2e: fix expected `client block` output
  rm block hash that was always empty
  • Loading branch information
tzemanovic committed Apr 26, 2024
2 parents 2268b7d + c2eccab commit 569c3c5
Show file tree
Hide file tree
Showing 34 changed files with 82 additions and 415 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/bug-fixes/3136-rm-empty-block-hash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Removed block hash and all the associated functions that were using it.
([\#3136](https://github.com/anoma/namada/pull/3136))
3 changes: 1 addition & 2 deletions crates/apps/src/lib/bench_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use masp_proofs::prover::LocalTxProver;
use namada::address::MASP;
use namada::core::address::{self, Address, InternalAddress};
use namada::core::chain::ChainId;
use namada::core::hash::Hash;
use namada::core::key::common::SecretKey;
use namada::core::masp::{
ExtendedViewingKey, PaymentAddress, TransferSource, TransferTarget,
Expand Down Expand Up @@ -571,7 +570,7 @@ impl BenchShell {
self.inner
.state
.in_mem_mut()
.begin_block(Hash::default().into(), last_height + 1)
.begin_block(last_height + 1)
.unwrap();

self.inner.commit();
Expand Down
3 changes: 1 addition & 2 deletions crates/apps/src/lib/client/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ pub async fn query_block(context: &impl Namada) {
Some(block) => {
display_line!(
context.io(),
"Last committed block ID: {}, height: {}, time: {}",
block.hash,
"Last committed block height: {}, time: {}",
block.height,
block.time
);
Expand Down
21 changes: 8 additions & 13 deletions crates/apps/src/lib/node/ledger/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use data_encoding::HEXUPPER;
use masp_primitives::merkle_tree::CommitmentTree;
use masp_primitives::sapling::Node;
use namada::core::storage::{BlockHash, BlockResults, Epoch, Header};
use namada::core::storage::{BlockResults, Epoch, Header};
use namada::gas::event::WithGasUsed;
use namada::governance::pgf::inflation as pgf_inflation;
use namada::hash::Hash;
Expand Down Expand Up @@ -63,7 +63,7 @@ where
let mut response = shim::response::FinalizeBlock::default();

// Begin the new block and check if a new epoch has begun
let (height, new_epoch) = self.update_state(req.header, req.hash);
let (height, new_epoch) = self.update_state(req.header);

let (current_epoch, _gas) = self.state.in_mem().get_current_epoch();
let update_for_tendermint = matches!(
Expand Down Expand Up @@ -540,21 +540,16 @@ where
Ok(response)
}

/// Sets the metadata necessary for a new block, including
/// the hash, height, validator changes, and evidence of
/// byzantine behavior. Applies slashes if necessary.
/// Returns a bool indicating if a new epoch began and
/// the height of the new block.
fn update_state(
&mut self,
header: Header,
hash: BlockHash,
) -> (BlockHeight, bool) {
/// Sets the metadata necessary for a new block, including the height,
/// validator changes, and evidence of byzantine behavior. Applies slashes
/// if necessary. Returns a bool indicating if a new epoch began and the
/// height of the new block.
fn update_state(&mut self, header: Header) -> (BlockHeight, bool) {
let height = self.state.in_mem().get_last_block_height() + 1;

self.state
.in_mem_mut()
.begin_block(hash, height)
.begin_block(height)
.expect("Beginning a block shouldn't fail");

let header_time = header.time;
Expand Down
3 changes: 1 addition & 2 deletions crates/apps/src/lib/node/ledger/shell/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ mod test_utils {
use namada::core::hash::Hash;
use namada::core::keccak::KeccakHash;
use namada::core::key::*;
use namada::core::storage::{BlockHash, Epoch, Header};
use namada::core::storage::{Epoch, Header};
use namada::proof_of_stake::parameters::PosParams;
use namada::proof_of_stake::storage::validator_consensus_key_handle;
use namada::state::mockdb::MockDB;
Expand Down Expand Up @@ -1880,7 +1880,6 @@ mod test_utils {
impl Default for FinalizeBlock {
fn default() -> Self {
FinalizeBlock {
hash: BlockHash([0u8; 32]),
header: Header {
hash: Hash([0; 32]),
#[allow(clippy::disallowed_methods)]
Expand Down
8 changes: 5 additions & 3 deletions crates/apps/src/lib/node/ledger/shell/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ where
// access to the `Shell` there
#[cfg(test)]
mod test_queries {
use namada::core::storage::{BlockHash, Epoch};
use namada::core::storage::Epoch;
use namada::eth_bridge::storage::eth_bridge_queries::is_bridge_comptime_enabled;
use namada::ledger::pos::PosQueries;
use namada::proof_of_stake::storage::read_consensus_validator_set_addresses_with_stake;
Expand Down Expand Up @@ -95,8 +95,10 @@ mod test_queries {
for (curr_epoch, curr_block_height, can_send) in
epoch_assertions
{
shell.state.in_mem_mut().begin_block(
BlockHash::default(), curr_block_height.into()).unwrap();
shell.state
.in_mem_mut()
.begin_block(curr_block_height.into())
.unwrap();

if prev_epoch != Some(curr_epoch) {
prev_epoch = Some(curr_epoch);
Expand Down
15 changes: 2 additions & 13 deletions crates/apps/src/lib/node/ledger/shell/testing/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use namada::core::ethereum_events::EthereumEvent;
use namada::core::ethereum_structs;
use namada::core::hash::Hash;
use namada::core::key::tm_consensus_key_raw_hash;
use namada::core::storage::{BlockHash, BlockHeight, Epoch, Header};
use namada::core::storage::{BlockHeight, Epoch, Header};
use namada::core::time::DateTimeUtc;
use namada::eth_bridge::oracle::config::Config as OracleConfig;
use namada::ledger::dry_run_tx;
Expand Down Expand Up @@ -442,7 +442,6 @@ impl MockNode {
};
// build finalize block abci request
let req = FinalizeBlock {
hash: BlockHash([0u8; 32]),
header: Header {
hash: Hash([0; 32]),
#[allow(clippy::disallowed_methods)]
Expand Down Expand Up @@ -560,7 +559,6 @@ impl MockNode {

// process proposal succeeded, now run finalize block
let req = FinalizeBlock {
hash: BlockHash([0u8; 32]),
header: Header {
hash: Hash([0; 32]),
#[allow(clippy::disallowed_methods)]
Expand Down Expand Up @@ -757,16 +755,7 @@ impl<'a> Client for &'a MockNode {
.map(|b| b.height.0 as u32)
.unwrap_or_default()
.into(),
last_block_app_hash: locked
.state
.in_mem()
.last_block
.as_ref()
.map(|b| b.hash.0)
.unwrap_or_default()
.to_vec()
.try_into()
.unwrap(),
last_block_app_hash: tendermint::AppHash::default(),
})
}

Expand Down
4 changes: 1 addition & 3 deletions crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::task::{Context, Poll};
use futures::future::FutureExt;
use namada::core::hash::Hash;
use namada::core::key::tm_raw_hash_to_string;
use namada::core::storage::{BlockHash, BlockHeight};
use namada::core::storage::BlockHeight;
use namada::proof_of_stake::storage::find_validator_by_raw_hash;
use namada::time::{DateTimeUtc, Utc};
use namada::tx::data::hash_tx;
Expand Down Expand Up @@ -146,8 +146,6 @@ impl AbcippShim {
}
let mut end_block_request: FinalizeBlock =
begin_block_request.into();
let hash = self.get_hash();
end_block_request.hash = BlockHash::from(hash);
end_block_request.txs = txs;
self.service
.call(Request::FinalizeBlock(end_block_request))
Expand Down
4 changes: 1 addition & 3 deletions crates/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ pub mod shim {
pub mod request {

use namada::core::hash::Hash;
use namada::core::storage::{BlockHash, Header};
use namada::core::storage::Header;
use namada::core::time::DateTimeUtc;

use super::VoteInfo;
Expand All @@ -171,7 +171,6 @@ pub mod shim {

#[derive(Debug, Clone)]
pub struct FinalizeBlock {
pub hash: BlockHash,
pub header: Header,
pub byzantine_validators: Vec<Misbehavior>,
pub txs: Vec<ProcessedTx>,
Expand All @@ -183,7 +182,6 @@ pub mod shim {
fn from(req: tm_request::BeginBlock) -> FinalizeBlock {
let header = req.header;
FinalizeBlock {
hash: BlockHash::default(),
header: Header {
hash: Hash::try_from(header.app_hash.as_bytes())
.unwrap_or_default(),
Expand Down
25 changes: 10 additions & 15 deletions crates/apps/src/lib/node/ledger/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ mod tests {
use namada::core::ethereum_events::Uint;
use namada::core::hash::Hash;
use namada::core::keccak::KeccakHash;
use namada::core::storage::{BlockHash, BlockHeight, Key};
use namada::core::storage::{BlockHeight, Key};
use namada::core::time::DurationSecs;
use namada::core::{address, storage};
use namada::eth_bridge::storage::proof::BridgePoolRootProof;
Expand Down Expand Up @@ -146,7 +146,7 @@ mod tests {
);
state
.in_mem_mut()
.begin_block(BlockHash::default(), BlockHeight(100))
.begin_block(BlockHeight(100))
.expect("begin_block failed");
state
.in_mem_mut()
Expand Down Expand Up @@ -193,7 +193,6 @@ mod tests {

// save the last state and the storage
let root = state.in_mem().merkle_root().0;
let hash = state.in_mem().get_block_hash().0;
let address_gen = state.in_mem().address_gen.clone();

// Release DB lock
Expand All @@ -212,7 +211,6 @@ mod tests {
state.in_mem().get_state().expect("no block exists");
assert_eq!(loaded_root.0, root);
assert_eq!(height, 100);
assert_eq!(state.in_mem().get_block_hash().0, hash);
assert_eq!(state.in_mem().address_gen, address_gen);
let (val, _) = state.db_read(&key).expect("read failed");
assert_eq!(val.expect("no value"), value_bytes);
Expand Down Expand Up @@ -276,7 +274,7 @@ mod tests {
);
state
.in_mem_mut()
.begin_block(BlockHash::default(), BlockHeight(100))
.begin_block(BlockHeight(100))
.expect("begin_block failed");

let addr = state
Expand Down Expand Up @@ -364,8 +362,7 @@ mod tests {

let key = Key::parse("key").expect("cannot parse the key string");
for (height, write_value) in blocks_write_value.clone() {
let hash = BlockHash::default();
state.in_mem_mut().begin_block(hash, height)?;
state.in_mem_mut().begin_block(height)?;
assert_eq!(
height,
state.in_mem().block.height,
Expand Down Expand Up @@ -468,9 +465,8 @@ mod tests {
state.db_write(&key, bytes)?;

// Update and commit
let hash = BlockHash::default();
let height = BlockHeight(1);
state.in_mem_mut().begin_block(hash, height)?;
state.in_mem_mut().begin_block(height)?;
// Epoch 0
state.in_mem_mut().block.pred_epochs.new_epoch(height);
let mut batch = PersistentState::batch();
Expand All @@ -489,9 +485,8 @@ mod tests {
state.in_mem().block.height,
state.in_mem().merkle_root(),
);
let hash = BlockHash::default();
let next_height = state.in_mem().block.height.next_height();
state.in_mem_mut().begin_block(hash, next_height)?;
state.in_mem_mut().begin_block(next_height)?;
batch = PersistentState::batch();
}
match write_type {
Expand Down Expand Up @@ -572,7 +567,7 @@ mod tests {

state
.in_mem_mut()
.begin_block(BlockHash::default(), new_epoch_start)
.begin_block(new_epoch_start)
.expect("begin_block failed");

let key = ibc_key("key").unwrap();
Expand All @@ -590,7 +585,7 @@ mod tests {
let new_epoch_start = BlockHeight(6);
state
.in_mem_mut()
.begin_block(BlockHash::default(), new_epoch_start)
.begin_block(new_epoch_start)
.expect("begin_block failed");

let key = ibc_key("key2").unwrap();
Expand All @@ -615,7 +610,7 @@ mod tests {
let new_epoch_start = BlockHeight(11);
state
.in_mem_mut()
.begin_block(BlockHash::default(), new_epoch_start)
.begin_block(new_epoch_start)
.expect("begin_block failed");

let nonce = nonce + 1;
Expand Down Expand Up @@ -648,7 +643,7 @@ mod tests {

state
.in_mem_mut()
.begin_block(BlockHash::default(), BlockHeight(12))
.begin_block(BlockHeight(12))
.expect("begin_block failed");

let nonce = nonce + 1;
Expand Down
17 changes: 1 addition & 16 deletions crates/apps/src/lib/node/ledger/storage/rocksdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
//! - `tree`: merkle tree
//! - `root`: root hash
//! - `store`: the tree's store
//! - `hash`: block hash
//! - `time`: block time
//! - `epoch`: block epoch
//! - `address_gen`: established address generator
Expand Down Expand Up @@ -103,7 +102,6 @@ const PRED_KEY_PREFIX: &str = "pred";
const MERKLE_TREE_ROOT_KEY_SEGMENT: &str = "root";
const MERKLE_TREE_STORE_KEY_SEGMENT: &str = "store";
const BLOCK_HEADER_KEY_SEGMENT: &str = "header";
const BLOCK_HASH_KEY_SEGMENT: &str = "hash";
const BLOCK_TIME_KEY_SEGMENT: &str = "time";
const EPOCH_KEY_SEGMENT: &str = "epoch";
const PRED_EPOCHS_KEY_SEGMENT: &str = "pred_epochs";
Expand Down Expand Up @@ -780,12 +778,6 @@ impl DB for RocksDB {

// Resotring the Mekle tree later

let hash_key = format!("{prefix}/{BLOCK_HASH_KEY_SEGMENT}");
let hash = match self.read_value(block_cf, hash_key)? {
Some(h) => h,
None => return Ok(None),
};

let time_key = format!("{prefix}/{BLOCK_TIME_KEY_SEGMENT}");
let time = match self.read_value(block_cf, time_key)? {
Some(t) => t,
Expand All @@ -811,7 +803,6 @@ impl DB for RocksDB {
};

Ok(Some(BlockStateRead {
hash,
height,
time,
epoch,
Expand All @@ -837,7 +828,6 @@ impl DB for RocksDB {
let BlockStateWrite {
merkle_tree_stores,
header,
hash,
height,
time,
epoch,
Expand Down Expand Up @@ -945,9 +935,6 @@ impl DB for RocksDB {
let header_key = format!("{prefix}/{BLOCK_HEADER_KEY_SEGMENT}");
self.add_value_to_batch(block_cf, header_key, &h, batch);
}
// Block hash
let hash_key = format!("{prefix}/{BLOCK_HASH_KEY_SEGMENT}");
self.add_value_to_batch(block_cf, hash_key, &hash, batch);
// Block time
let time_key = format!("{prefix}/{BLOCK_TIME_KEY_SEGMENT}");
self.add_value_to_batch(block_cf, time_key, &time, batch);
Expand Down Expand Up @@ -1807,7 +1794,7 @@ mod imp {
mod test {
use namada::address::EstablishedAddressGen;
use namada::core::hash::Hash;
use namada::core::storage::{BlockHash, Epochs};
use namada::core::storage::Epochs;
use namada::ledger::storage::ConversionState;
use namada::state::{MerkleTree, Sha256Hasher};
use namada::storage::{BlockResults, EthEventsQueue};
Expand Down Expand Up @@ -2335,7 +2322,6 @@ mod test {
) -> Result<()> {
let merkle_tree = MerkleTree::<Sha256Hasher>::default();
let merkle_tree_stores = merkle_tree.stores();
let hash = BlockHash::default();
#[allow(clippy::disallowed_methods)]
let time = DateTimeUtc::now();
let next_epoch_min_start_height = BlockHeight::default();
Expand All @@ -2349,7 +2335,6 @@ mod test {
let block = BlockStateWrite {
merkle_tree_stores,
header: None,
hash: &hash,
height,
time,
epoch,
Expand Down
Loading

0 comments on commit 569c3c5

Please sign in to comment.