return errors in state methods (#2417)

This commit is contained in:
Conrado Gouvea 2021-06-30 00:52:31 -03:00 committed by GitHub
parent b68202c68a
commit c101c12e5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 106 additions and 66 deletions

View File

@ -178,9 +178,9 @@ impl StateService {
let parent_hash = prepared.block.header.previous_block_hash; let parent_hash = prepared.block.header.previous_block_hash;
if self.disk.finalized_tip_hash() == parent_hash { if self.disk.finalized_tip_hash() == parent_hash {
self.mem.commit_new_chain(prepared); self.mem.commit_new_chain(prepared)?;
} else { } else {
self.mem.commit_block(prepared); self.mem.commit_block(prepared)?;
} }
Ok(()) Ok(())

View File

@ -19,7 +19,7 @@ use zebra_chain::{
transparent, transparent,
}; };
use crate::{FinalizedBlock, HashOrHeight, PreparedBlock, Utxo}; use crate::{FinalizedBlock, HashOrHeight, PreparedBlock, Utxo, ValidateContextError};
use self::chain::Chain; use self::chain::Chain;
@ -74,7 +74,7 @@ impl NonFinalizedState {
} }
/// Commit block to the non-finalized state. /// Commit block to the non-finalized state.
pub fn commit_block(&mut self, prepared: PreparedBlock) { pub fn commit_block(&mut self, prepared: PreparedBlock) -> Result<(), ValidateContextError> {
let parent_hash = prepared.block.header.previous_block_hash; let parent_hash = prepared.block.header.previous_block_hash;
let (height, hash) = (prepared.height, prepared.hash); let (height, hash) = (prepared.height, prepared.hash);
@ -85,29 +85,26 @@ impl NonFinalizedState {
); );
} }
let mut parent_chain = self let mut parent_chain = self.parent_chain(parent_hash)?;
.take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash)
.or_else(|| {
self.chain_set
.iter()
.find_map(|chain| chain.fork(parent_hash))
.map(Box::new)
})
.expect("commit_block is only called with blocks that are ready to be commited");
parent_chain.push(prepared); parent_chain.push(prepared)?;
self.chain_set.insert(parent_chain); self.chain_set.insert(parent_chain);
self.update_metrics_for_committed_block(height, hash); self.update_metrics_for_committed_block(height, hash);
Ok(())
} }
/// Commit block to the non-finalized state as a new chain where its parent /// Commit block to the non-finalized state as a new chain where its parent
/// is the finalized tip. /// is the finalized tip.
pub fn commit_new_chain(&mut self, prepared: PreparedBlock) { pub fn commit_new_chain(
&mut self,
prepared: PreparedBlock,
) -> Result<(), ValidateContextError> {
let mut chain = Chain::default(); let mut chain = Chain::default();
let (height, hash) = (prepared.height, prepared.hash); let (height, hash) = (prepared.height, prepared.hash);
chain.push(prepared); chain.push(prepared)?;
self.chain_set.insert(Box::new(chain)); self.chain_set.insert(Box::new(chain));
self.update_metrics_for_committed_block(height, hash); self.update_metrics_for_committed_block(height, hash);
Ok(())
} }
/// Returns the length of the non-finalized portion of the current best chain. /// Returns the length of the non-finalized portion of the current best chain.
@ -245,6 +242,29 @@ impl NonFinalizedState {
.map(|box_chain| box_chain.deref()) .map(|box_chain| box_chain.deref())
} }
/// Return the chain whose tip block hash is `parent_hash`.
///
/// The chain can be an existing chain in the non-finalized state or a freshly
/// created fork, if needed.
fn parent_chain(
&mut self,
parent_hash: block::Hash,
) -> Result<Box<Chain>, ValidateContextError> {
match self.take_chain_if(|chain| chain.non_finalized_tip_hash() == parent_hash) {
// An existing chain in the non-finalized state
Some(chain) => Ok(chain),
// Create a new fork
None => Ok(Box::new(
self.chain_set
.iter()
.find_map(|chain| chain.fork(parent_hash).transpose())
.expect(
"commit_block is only called with blocks that are ready to be commited",
)?,
)),
}
}
/// Update the metrics after `block` is committed /// Update the metrics after `block` is committed
fn update_metrics_for_committed_block(&self, height: block::Height, hash: block::Hash) { fn update_metrics_for_committed_block(&self, height: block::Height, hash: block::Hash) {
metrics::counter!("state.memory.committed.block.count", 1); metrics::counter!("state.memory.committed.block.count", 1);

View File

@ -10,7 +10,7 @@ use zebra_chain::{
transaction::Transaction::*, transparent, work::difficulty::PartialCumulativeWork, transaction::Transaction::*, transparent, work::difficulty::PartialCumulativeWork,
}; };
use crate::{PreparedBlock, Utxo}; use crate::{PreparedBlock, Utxo, ValidateContextError};
#[derive(Default, Clone)] #[derive(Default, Clone)]
pub struct Chain { pub struct Chain {
@ -32,11 +32,12 @@ pub struct Chain {
impl Chain { impl Chain {
/// Push a contextually valid non-finalized block into a chain as the new tip. /// Push a contextually valid non-finalized block into a chain as the new tip.
#[instrument(level = "debug", skip(self, block), fields(block = %block.block))] #[instrument(level = "debug", skip(self, block), fields(block = %block.block))]
pub fn push(&mut self, block: PreparedBlock) { pub fn push(&mut self, block: PreparedBlock) -> Result<(), ValidateContextError> {
// update cumulative data members // update cumulative data members
self.update_chain_state_with(&block); self.update_chain_state_with(&block)?;
tracing::debug!(block = %block.block, "adding block to chain"); tracing::debug!(block = %block.block, "adding block to chain");
self.blocks.insert(block.height, block); self.blocks.insert(block.height, block);
Ok(())
} }
/// Remove the lowest height block of the non-finalized portion of a chain. /// Remove the lowest height block of the non-finalized portion of a chain.
@ -67,9 +68,9 @@ impl Chain {
/// Fork a chain at the block with the given hash, if it is part of this /// Fork a chain at the block with the given hash, if it is part of this
/// chain. /// chain.
pub fn fork(&self, fork_tip: block::Hash) -> Option<Self> { pub fn fork(&self, fork_tip: block::Hash) -> Result<Option<Self>, ValidateContextError> {
if !self.height_by_hash.contains_key(&fork_tip) { if !self.height_by_hash.contains_key(&fork_tip) {
return None; return Ok(None);
} }
let mut forked = self.clone(); let mut forked = self.clone();
@ -78,7 +79,7 @@ impl Chain {
forked.pop_tip(); forked.pop_tip();
} }
Some(forked) Ok(Some(forked))
} }
pub fn non_finalized_tip_hash(&self) -> block::Hash { pub fn non_finalized_tip_hash(&self) -> block::Hash {
@ -129,7 +130,7 @@ impl Chain {
trait UpdateWith<T> { trait UpdateWith<T> {
/// Update `Chain` cumulative data members to add data that are derived from /// Update `Chain` cumulative data members to add data that are derived from
/// `T` /// `T`
fn update_chain_state_with(&mut self, _: &T); fn update_chain_state_with(&mut self, _: &T) -> Result<(), ValidateContextError>;
/// Update `Chain` cumulative data members to remove data that are derived /// Update `Chain` cumulative data members to remove data that are derived
/// from `T` /// from `T`
@ -137,7 +138,10 @@ trait UpdateWith<T> {
} }
impl UpdateWith<PreparedBlock> for Chain { impl UpdateWith<PreparedBlock> for Chain {
fn update_chain_state_with(&mut self, prepared: &PreparedBlock) { fn update_chain_state_with(
&mut self,
prepared: &PreparedBlock,
) -> Result<(), ValidateContextError> {
let (block, hash, height, transaction_hashes) = ( let (block, hash, height, transaction_hashes) = (
prepared.block.as_ref(), prepared.block.as_ref(),
prepared.hash, prepared.hash,
@ -207,16 +211,18 @@ impl UpdateWith<PreparedBlock> for Chain {
); );
// add the utxos this produced // add the utxos this produced
self.update_chain_state_with(&prepared.new_outputs); self.update_chain_state_with(&prepared.new_outputs)?;
// add the utxos this consumed // add the utxos this consumed
self.update_chain_state_with(inputs); self.update_chain_state_with(inputs)?;
// add the shielded data // add the shielded data
self.update_chain_state_with(joinsplit_data); self.update_chain_state_with(joinsplit_data)?;
self.update_chain_state_with(sapling_shielded_data_per_spend_anchor); self.update_chain_state_with(sapling_shielded_data_per_spend_anchor)?;
self.update_chain_state_with(sapling_shielded_data_shared_anchor); self.update_chain_state_with(sapling_shielded_data_shared_anchor)?;
self.update_chain_state_with(orchard_shielded_data); self.update_chain_state_with(orchard_shielded_data)?;
} }
Ok(())
} }
#[instrument(skip(self, prepared), fields(block = %prepared.block))] #[instrument(skip(self, prepared), fields(block = %prepared.block))]
@ -296,9 +302,13 @@ impl UpdateWith<PreparedBlock> for Chain {
} }
impl UpdateWith<HashMap<transparent::OutPoint, Utxo>> for Chain { impl UpdateWith<HashMap<transparent::OutPoint, Utxo>> for Chain {
fn update_chain_state_with(&mut self, utxos: &HashMap<transparent::OutPoint, Utxo>) { fn update_chain_state_with(
&mut self,
utxos: &HashMap<transparent::OutPoint, Utxo>,
) -> Result<(), ValidateContextError> {
self.created_utxos self.created_utxos
.extend(utxos.iter().map(|(k, v)| (*k, v.clone()))); .extend(utxos.iter().map(|(k, v)| (*k, v.clone())));
Ok(())
} }
fn revert_chain_state_with(&mut self, utxos: &HashMap<transparent::OutPoint, Utxo>) { fn revert_chain_state_with(&mut self, utxos: &HashMap<transparent::OutPoint, Utxo>) {
@ -308,7 +318,10 @@ impl UpdateWith<HashMap<transparent::OutPoint, Utxo>> for Chain {
} }
impl UpdateWith<Vec<transparent::Input>> for Chain { impl UpdateWith<Vec<transparent::Input>> for Chain {
fn update_chain_state_with(&mut self, inputs: &Vec<transparent::Input>) { fn update_chain_state_with(
&mut self,
inputs: &Vec<transparent::Input>,
) -> Result<(), ValidateContextError> {
for consumed_utxo in inputs { for consumed_utxo in inputs {
match consumed_utxo { match consumed_utxo {
transparent::Input::PrevOut { outpoint, .. } => { transparent::Input::PrevOut { outpoint, .. } => {
@ -317,6 +330,7 @@ impl UpdateWith<Vec<transparent::Input>> for Chain {
transparent::Input::Coinbase { .. } => {} transparent::Input::Coinbase { .. } => {}
} }
} }
Ok(())
} }
fn revert_chain_state_with(&mut self, inputs: &Vec<transparent::Input>) { fn revert_chain_state_with(&mut self, inputs: &Vec<transparent::Input>) {
@ -339,7 +353,7 @@ impl UpdateWith<Option<transaction::JoinSplitData<Groth16Proof>>> for Chain {
fn update_chain_state_with( fn update_chain_state_with(
&mut self, &mut self,
joinsplit_data: &Option<transaction::JoinSplitData<Groth16Proof>>, joinsplit_data: &Option<transaction::JoinSplitData<Groth16Proof>>,
) { ) -> Result<(), ValidateContextError> {
if let Some(joinsplit_data) = joinsplit_data { if let Some(joinsplit_data) = joinsplit_data {
for sprout::JoinSplit { nullifiers, .. } in joinsplit_data.joinsplits() { for sprout::JoinSplit { nullifiers, .. } in joinsplit_data.joinsplits() {
let span = debug_span!("revert_chain_state_with", ?nullifiers); let span = debug_span!("revert_chain_state_with", ?nullifiers);
@ -349,6 +363,7 @@ impl UpdateWith<Option<transaction::JoinSplitData<Groth16Proof>>> for Chain {
self.sprout_nullifiers.insert(nullifiers[1]); self.sprout_nullifiers.insert(nullifiers[1]);
} }
} }
Ok(())
} }
#[instrument(skip(self, joinsplit_data))] #[instrument(skip(self, joinsplit_data))]
@ -381,12 +396,13 @@ where
fn update_chain_state_with( fn update_chain_state_with(
&mut self, &mut self,
sapling_shielded_data: &Option<sapling::ShieldedData<AnchorV>>, sapling_shielded_data: &Option<sapling::ShieldedData<AnchorV>>,
) { ) -> Result<(), ValidateContextError> {
if let Some(sapling_shielded_data) = sapling_shielded_data { if let Some(sapling_shielded_data) = sapling_shielded_data {
for nullifier in sapling_shielded_data.nullifiers() { for nullifier in sapling_shielded_data.nullifiers() {
self.sapling_nullifiers.insert(*nullifier); self.sapling_nullifiers.insert(*nullifier);
} }
} }
Ok(())
} }
fn revert_chain_state_with( fn revert_chain_state_with(
@ -405,12 +421,16 @@ where
} }
impl UpdateWith<Option<orchard::ShieldedData>> for Chain { impl UpdateWith<Option<orchard::ShieldedData>> for Chain {
fn update_chain_state_with(&mut self, orchard_shielded_data: &Option<orchard::ShieldedData>) { fn update_chain_state_with(
&mut self,
orchard_shielded_data: &Option<orchard::ShieldedData>,
) -> Result<(), ValidateContextError> {
if let Some(orchard_shielded_data) = orchard_shielded_data { if let Some(orchard_shielded_data) = orchard_shielded_data {
for nullifier in orchard_shielded_data.nullifiers() { for nullifier in orchard_shielded_data.nullifiers() {
self.orchard_nullifiers.insert(*nullifier); self.orchard_nullifiers.insert(*nullifier);
} }
} }
Ok(())
} }
fn revert_chain_state_with(&mut self, orchard_shielded_data: &Option<orchard::ShieldedData>) { fn revert_chain_state_with(&mut self, orchard_shielded_data: &Option<orchard::ShieldedData>) {

View File

@ -20,13 +20,13 @@ fn forked_equals_pushed() -> Result<()> {
let mut partial_chain = Chain::default(); let mut partial_chain = Chain::default();
for block in chain.iter().take(count) { for block in chain.iter().take(count) {
partial_chain.push(block.clone()); partial_chain.push(block.clone())?;
} }
for block in chain.iter() { for block in chain.iter() {
full_chain.push(block.clone()); full_chain.push(block.clone())?;
} }
let forked = full_chain.fork(fork_tip_hash).expect("hash is present"); let forked = full_chain.fork(fork_tip_hash).expect("fork works").expect("hash is present");
prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len()); prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len());
}); });
@ -48,10 +48,10 @@ fn finalized_equals_pushed() -> Result<()> {
let mut partial_chain = Chain::default(); let mut partial_chain = Chain::default();
for block in chain.iter().skip(finalized_count) { for block in chain.iter().skip(finalized_count) {
partial_chain.push(block.clone()); partial_chain.push(block.clone())?;
} }
for block in chain.iter() { for block in chain.iter() {
full_chain.push(block.clone()); full_chain.push(block.clone())?;
} }
for _ in 0..finalized_count { for _ in 0..finalized_count {

View File

@ -23,7 +23,7 @@ fn construct_single() -> Result<()> {
zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into()?; zebra_test::vectors::BLOCK_MAINNET_434873_BYTES.zcash_deserialize_into()?;
let mut chain = Chain::default(); let mut chain = Chain::default();
chain.push(block.prepare()); chain.push(block.prepare())?;
assert_eq!(1, chain.blocks.len()); assert_eq!(1, chain.blocks.len());
@ -47,7 +47,7 @@ fn construct_many() -> Result<()> {
let mut chain = Chain::default(); let mut chain = Chain::default();
for block in blocks { for block in blocks {
chain.push(block.prepare()); chain.push(block.prepare())?;
} }
assert_eq!(100, chain.blocks.len()); assert_eq!(100, chain.blocks.len());
@ -64,10 +64,10 @@ fn ord_matches_work() -> Result<()> {
let more_block = less_block.clone().set_work(10); let more_block = less_block.clone().set_work(10);
let mut lesser_chain = Chain::default(); let mut lesser_chain = Chain::default();
lesser_chain.push(less_block.prepare()); lesser_chain.push(less_block.prepare())?;
let mut bigger_chain = Chain::default(); let mut bigger_chain = Chain::default();
bigger_chain.push(more_block.prepare()); bigger_chain.push(more_block.prepare())?;
assert!(bigger_chain > lesser_chain); assert!(bigger_chain > lesser_chain);
@ -100,8 +100,8 @@ fn best_chain_wins_for_network(network: Network) -> Result<()> {
let expected_hash = block2.hash(); let expected_hash = block2.hash();
let mut state = NonFinalizedState::default(); let mut state = NonFinalizedState::default();
state.commit_new_chain(block2.prepare()); state.commit_new_chain(block2.prepare())?;
state.commit_new_chain(child.prepare()); state.commit_new_chain(child.prepare())?;
let best_chain = state.best_chain().unwrap(); let best_chain = state.best_chain().unwrap();
assert!(best_chain.height_by_hash.contains_key(&expected_hash)); assert!(best_chain.height_by_hash.contains_key(&expected_hash));
@ -133,9 +133,9 @@ fn finalize_pops_from_best_chain_for_network(network: Network) -> Result<()> {
let child = block1.make_fake_child().set_work(1); let child = block1.make_fake_child().set_work(1);
let mut state = NonFinalizedState::default(); let mut state = NonFinalizedState::default();
state.commit_new_chain(block1.clone().prepare()); state.commit_new_chain(block1.clone().prepare())?;
state.commit_block(block2.clone().prepare()); state.commit_block(block2.clone().prepare())?;
state.commit_block(child.prepare()); state.commit_block(child.prepare())?;
let finalized = state.finalize(); let finalized = state.finalize();
assert_eq!(block1, finalized.block); assert_eq!(block1, finalized.block);
@ -177,13 +177,13 @@ fn commit_block_extending_best_chain_doesnt_drop_worst_chains_for_network(
let mut state = NonFinalizedState::default(); let mut state = NonFinalizedState::default();
assert_eq!(0, state.chain_set.len()); assert_eq!(0, state.chain_set.len());
state.commit_new_chain(block1.prepare()); state.commit_new_chain(block1.prepare())?;
assert_eq!(1, state.chain_set.len()); assert_eq!(1, state.chain_set.len());
state.commit_block(block2.prepare()); state.commit_block(block2.prepare())?;
assert_eq!(1, state.chain_set.len()); assert_eq!(1, state.chain_set.len());
state.commit_block(child1.prepare()); state.commit_block(child1.prepare())?;
assert_eq!(2, state.chain_set.len()); assert_eq!(2, state.chain_set.len());
state.commit_block(child2.prepare()); state.commit_block(child2.prepare())?;
assert_eq!(2, state.chain_set.len()); assert_eq!(2, state.chain_set.len());
Ok(()) Ok(())
@ -215,10 +215,10 @@ fn shorter_chain_can_be_best_chain_for_network(network: Network) -> Result<()> {
let short_chain_block = block1.make_fake_child().set_work(3); let short_chain_block = block1.make_fake_child().set_work(3);
let mut state = NonFinalizedState::default(); let mut state = NonFinalizedState::default();
state.commit_new_chain(block1.prepare()); state.commit_new_chain(block1.prepare())?;
state.commit_block(long_chain_block1.prepare()); state.commit_block(long_chain_block1.prepare())?;
state.commit_block(long_chain_block2.prepare()); state.commit_block(long_chain_block2.prepare())?;
state.commit_block(short_chain_block.prepare()); state.commit_block(short_chain_block.prepare())?;
assert_eq!(2, state.chain_set.len()); assert_eq!(2, state.chain_set.len());
assert_eq!(2, state.best_chain_len()); assert_eq!(2, state.best_chain_len());
@ -254,12 +254,12 @@ fn longer_chain_with_more_work_wins_for_network(network: Network) -> Result<()>
let short_chain_block = block1.make_fake_child().set_work(3); let short_chain_block = block1.make_fake_child().set_work(3);
let mut state = NonFinalizedState::default(); let mut state = NonFinalizedState::default();
state.commit_new_chain(block1.prepare()); state.commit_new_chain(block1.prepare())?;
state.commit_block(long_chain_block1.prepare()); state.commit_block(long_chain_block1.prepare())?;
state.commit_block(long_chain_block2.prepare()); state.commit_block(long_chain_block2.prepare())?;
state.commit_block(long_chain_block3.prepare()); state.commit_block(long_chain_block3.prepare())?;
state.commit_block(long_chain_block4.prepare()); state.commit_block(long_chain_block4.prepare())?;
state.commit_block(short_chain_block.prepare()); state.commit_block(short_chain_block.prepare())?;
assert_eq!(2, state.chain_set.len()); assert_eq!(2, state.chain_set.len());
assert_eq!(5, state.best_chain_len()); assert_eq!(5, state.best_chain_len());
@ -291,9 +291,9 @@ fn equal_length_goes_to_more_work_for_network(network: Network) -> Result<()> {
let expected_hash = more_work_child.hash(); let expected_hash = more_work_child.hash();
let mut state = NonFinalizedState::default(); let mut state = NonFinalizedState::default();
state.commit_new_chain(block1.prepare()); state.commit_new_chain(block1.prepare())?;
state.commit_block(less_work_child.prepare()); state.commit_block(less_work_child.prepare())?;
state.commit_block(more_work_child.prepare()); state.commit_block(more_work_child.prepare())?;
assert_eq!(2, state.chain_set.len()); assert_eq!(2, state.chain_set.len());
let tip_hash = state.best_tip().unwrap().1; let tip_hash = state.best_tip().unwrap().1;