Skip to content

Commit

Permalink
Merge pull request #851 from anoma/bat/ethbridge/proof-values-ref
Browse files Browse the repository at this point in the history
 Changed the bytes passed into merkle proof to be references
  • Loading branch information
batconjurer authored Dec 6, 2022
2 parents b3c4ff7 + 2af693a commit ca0cb2d
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 53 deletions.
10 changes: 0 additions & 10 deletions proof_of_stake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1046,8 +1046,6 @@ pub enum BondError<Address: Display + Debug> {
InactiveValidator(Address),
#[error("Voting power overflow: {0}")]
VotingPowerOverflow(TryFromIntError),
#[error("Given zero amount to bond")]
ZeroAmount,
}

#[allow(missing_docs)]
Expand All @@ -1065,8 +1063,6 @@ pub enum UnbondError<Address: Display + Debug, TokenAmount: Display + Debug> {
ValidatorHasNoVotingPower(Address),
#[error("Voting power overflow: {0}")]
VotingPowerOverflow(TryFromIntError),
#[error("Given zero amount to unbond")]
ZeroAmount,
}

#[allow(missing_docs)]
Expand Down Expand Up @@ -1611,9 +1607,6 @@ where
+ BorshSerialize
+ BorshSchema,
{
if amount == TokenAmount::default() {
return Err(BondError::ZeroAmount);
}
// Check the validator state
match validator_state {
None => {
Expand Down Expand Up @@ -1791,9 +1784,6 @@ where
+ BorshSerialize
+ BorshSchema,
{
if amount == TokenAmount::default() {
return Err(UnbondError::ZeroAmount);
}
// We can unbond tokens that are bonded for a future epoch (not yet
// active), hence we check the total at the pipeline offset
let unbondable_amount = bond
Expand Down
13 changes: 6 additions & 7 deletions shared/src/ledger/queries/shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,7 @@ where
let proof = if request.prove {
let proof = ctx
.storage
.get_existence_proof(
&storage_key,
value.clone(),
request.height,
)
.get_existence_proof(&storage_key, &value, request.height)
.into_storage_result()?;
Some(proof)
} else {
Expand Down Expand Up @@ -309,7 +305,7 @@ where
for PrefixValue { key, value } in &data {
let mut proof = ctx
.storage
.get_existence_proof(key, value.clone(), request.height)
.get_existence_proof(key, value, request.height)
.into_storage_result()?;
ops.append(&mut proof.ops);
}
Expand Down Expand Up @@ -477,7 +473,10 @@ where
)));
}
// get the membership proof
match tree.get_sub_tree_existence_proof(&keys, values) {
match tree.get_sub_tree_existence_proof(
&keys,
values.iter().map(|v| v.as_slice()).collect(),
) {
Ok(BridgePool(proof)) => {
let data = EncodeCell::new(&RelayProof {
// TODO: use actual validators
Expand Down
6 changes: 3 additions & 3 deletions shared/src/ledger/storage/merkle_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub enum Error {
type Result<T> = std::result::Result<T, Error>;

/// Type alias for bytes to be put into the Merkle storage
pub(super) type StorageBytes = Vec<u8>;
pub(super) type StorageBytes<'a> = &'a [u8];

/// Type aliases for the different merkle trees and backing stores
pub type SmtStore = DefaultStore<SmtHash, Hash, 32>;
Expand Down Expand Up @@ -733,7 +733,7 @@ mod test {
let proof = match tree
.get_sub_tree_existence_proof(
std::array::from_ref(&ibc_key),
vec![ibc_val.clone()],
vec![&ibc_val],
)
.unwrap()
{
Expand Down Expand Up @@ -792,7 +792,7 @@ mod test {
let proof = match tree
.get_sub_tree_existence_proof(
std::array::from_ref(&pos_key),
vec![pos_val.clone()],
vec![&pos_val],
)
.unwrap()
{
Expand Down
4 changes: 2 additions & 2 deletions shared/src/ledger/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use crate::ledger::pos::namada_proof_of_stake::PosBase;
use crate::ledger::pos::types::WeightedValidator;
use crate::ledger::pos::PosParams;
use crate::ledger::storage::merkle_tree::{
Error as MerkleTreeError, MerkleRoot,
Error as MerkleTreeError, MerkleRoot, StorageBytes,
};
pub use crate::ledger::storage::merkle_tree::{
MerkleTree, MerkleTreeStoresRead, MerkleTreeStoresWrite, StoreRef,
Expand Down Expand Up @@ -619,7 +619,7 @@ where
pub fn get_existence_proof(
&self,
key: &Key,
value: Vec<u8>,
value: StorageBytes,
height: BlockHeight,
) -> Result<Proof> {
if height >= self.get_block_height().0 {
Expand Down
18 changes: 10 additions & 8 deletions shared/src/ledger/storage/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pub trait SubTreeWrite {
fn subtree_update(
&mut self,
key: &Key,
value: &[u8],
value: StorageBytes,
) -> Result<Hash, Error>;
/// Delete a key from the sub-tree
fn subtree_delete(&mut self, key: &Key) -> Result<Hash, Error>;
Expand Down Expand Up @@ -68,7 +68,7 @@ impl<'a, H: StorageHasher + Default> SubTreeRead for &'a Smt<H> {
Ics23Proof::Exist(ep) => Ok(CommitmentProof {
proof: Some(Ics23Proof::Exist(ExistenceProof {
key: key.to_string().as_bytes().to_vec(),
value,
value: value.to_vec(),
leaf: Some(ics23_specs::leaf_spec::<H>()),
..ep
})),
Expand All @@ -84,7 +84,7 @@ impl<'a, H: StorageHasher + Default> SubTreeWrite for &'a mut Smt<H> {
fn subtree_update(
&mut self,
key: &Key,
value: &[u8],
value: StorageBytes,
) -> Result<Hash, Error> {
let value = H::hash(value);
self.update(H::hash(key.to_string()).into(), value.into())
Expand Down Expand Up @@ -139,7 +139,7 @@ impl<'a, H: StorageHasher + Default> SubTreeWrite for &'a mut Amt<H> {
fn subtree_update(
&mut self,
key: &Key,
value: &[u8],
value: StorageBytes,
) -> Result<Hash, Error> {
let key = StringKey::try_from_bytes(key.to_string().as_bytes())?;
let value = TreeBytes::from(value.as_ref().to_owned());
Expand Down Expand Up @@ -170,9 +170,7 @@ impl<'a> SubTreeRead for &'a BridgePoolTree {
) -> Result<MembershipProof, Error> {
let values = values
.iter()
.filter_map(|val| {
PendingTransfer::try_from_slice(val.as_slice()).ok()
})
.filter_map(|val| PendingTransfer::try_from_slice(val).ok())
.collect();
self.get_membership_proof(values)
.map(Into::into)
Expand All @@ -181,7 +179,11 @@ impl<'a> SubTreeRead for &'a BridgePoolTree {
}

impl<'a> SubTreeWrite for &'a mut BridgePoolTree {
fn subtree_update(&mut self, key: &Key, _: &[u8]) -> Result<Hash, Error> {
fn subtree_update(
&mut self,
key: &Key,
_: StorageBytes,
) -> Result<Hash, Error> {
self.insert_key(key)
.map_err(|err| Error::MerkleTree(err.to_string()))
}
Expand Down
10 changes: 9 additions & 1 deletion shared/src/types/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl FromStr for Amount {
match rust_decimal::Decimal::from_str(s) {
Ok(decimal) => {
let scale = decimal.scale();
if scale > 6 {
if scale > MAX_DECIMAL_PLACES {
return Err(AmountParseError::ScaleTooLarge(scale));
}
let whole =
Expand Down Expand Up @@ -518,4 +518,12 @@ pub mod testing {
pub fn arb_amount_ceiled(max: u64) -> impl Strategy<Value = Amount> {
(0..=max).prop_map(Amount::from)
}

/// Generate an arbitrary non-zero token amount up to and including given
/// `max` value
pub fn arb_amount_non_zero_ceiled(
max: u64,
) -> impl Strategy<Value = Amount> {
(1..=max).prop_map(Amount::from)
}
}
2 changes: 1 addition & 1 deletion wasm/wasm_source/proptest-regressions/tx_bond.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
cc e54347c5114ef29538127ba9ad68d1572af839ec63c015318fc0827818853a22
cc f22e874350910b197cb02a4a07ec5bef18e16c0d1a39eaabaee43d1fc05ce11d
2 changes: 1 addition & 1 deletion wasm/wasm_source/src/tx_bond.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ mod tests {
(
arb_established_address(),
prop::option::of(arb_non_internal_address()),
token::testing::arb_amount_ceiled(max_amount),
token::testing::arb_amount_non_zero_ceiled(max_amount),
)
.prop_map(|(validator, source, amount)| {
transaction::pos::Bond {
Expand Down
42 changes: 22 additions & 20 deletions wasm/wasm_source/src/tx_unbond.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,24 +225,24 @@ mod tests {
epoch {epoch}"
);
}
let start_epoch = match &unbond.source {
Some(_) => {
// This bond was a delegation
namada_tx_prelude::proof_of_stake::types::Epoch::from(
pos_params.pipeline_len,
)
}
None => {
// This bond was a genesis validator self-bond
namada_tx_prelude::proof_of_stake::types::Epoch::default()
}
let start_epoch = if is_delegation {
// This bond was a delegation
namada_tx_prelude::proof_of_stake::types::Epoch::from(
pos_params.pipeline_len,
)
} else {
// This bond was a genesis validator self-bond
namada_tx_prelude::proof_of_stake::types::Epoch::default()
};
let end_epoch = namada_tx_prelude::proof_of_stake::types::Epoch::from(
pos_params.unbonding_len - 1,
);

let expected_unbond =
HashMap::from_iter([((start_epoch, end_epoch), unbond.amount)]);
let expected_unbond = if unbond.amount == token::Amount::default() {
HashMap::new()
} else {
HashMap::from_iter([((start_epoch, end_epoch), unbond.amount)])
};
let actual_unbond: Unbond<token::Amount> =
unbonds_post.get(pos_params.unbonding_len).unwrap();
assert_eq!(
Expand Down Expand Up @@ -390,12 +390,14 @@ mod tests {
fn arb_initial_stake_and_unbond()
-> impl Strategy<Value = (token::Amount, transaction::pos::Unbond)> {
// Generate initial stake
token::testing::arb_amount().prop_flat_map(|initial_stake| {
// Use the initial stake to limit the bond amount
let unbond = arb_unbond(u64::from(initial_stake));
// Use the generated initial stake too too
(Just(initial_stake), unbond)
})
token::testing::arb_amount_ceiled((i64::MAX / 8) as u64).prop_flat_map(
|initial_stake| {
// Use the initial stake to limit the bond amount
let unbond = arb_unbond(u64::from(initial_stake));
// Use the generated initial stake too too
(Just(initial_stake), unbond)
},
)
}

/// Generates an initial validator stake and a unbond, while making sure
Expand All @@ -406,7 +408,7 @@ mod tests {
(
address::testing::arb_established_address(),
prop::option::of(address::testing::arb_non_internal_address()),
token::testing::arb_amount_ceiled(max_amount),
token::testing::arb_amount_non_zero_ceiled(max_amount),
)
.prop_map(|(validator, source, amount)| {
let validator = Address::Established(validator);
Expand Down

0 comments on commit ca0cb2d

Please sign in to comment.