Skip to content

Commit

Permalink
Protocol now ensures that a DenominatedAmounts denom is correct. Adde…
Browse files Browse the repository at this point in the history 
…d checked arithmetic operations to DenominatedAmount to catch mistakes.
  • Loading branch information
@murisi
murisi committed Dec 5, 2023
1 parent 32ec88c commit 95c5f6f
Show file tree
Hide file tree
Showing 46 changed files with 362 additions and 275 deletions.
22 changes: 13 additions & 9 deletions apps/src/lib/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2872,9 +2872,11 @@ pub mod args {
pub const BRIDGE_POOL_GAS_AMOUNT: ArgDefault<token::DenominatedAmount> =
arg_default(
"pool-gas-amount",
DefaultFn(|| token::DenominatedAmount {
amount: token::Amount::zero(),
denom: NATIVE_MAX_DECIMAL_PLACES.into(),
DefaultFn(|| {
token::DenominatedAmount::new(
token::Amount::zero(),
NATIVE_MAX_DECIMAL_PLACES.into(),
)
}),
);
pub const BRIDGE_POOL_GAS_PAYER: ArgOpt<WalletAddress> =
Expand Down Expand Up @@ -2944,9 +2946,11 @@ pub mod args {
pub const FEE_PAYER: Arg<WalletAddress> = arg("fee-payer");
pub const FEE_AMOUNT: ArgDefault<token::DenominatedAmount> = arg_default(
"fee-amount",
DefaultFn(|| token::DenominatedAmount {
amount: token::Amount::default(),
denom: NATIVE_MAX_DECIMAL_PLACES.into(),
DefaultFn(|| {
token::DenominatedAmount::new(
token::Amount::default(),
NATIVE_MAX_DECIMAL_PLACES.into(),
)
}),
);
pub const GENESIS_PATH: Arg<PathBuf> = arg("genesis-path");
Expand Down Expand Up @@ -4261,7 +4265,7 @@ pub mod args {
println!("Could not parse bond amount: {:?}", e);
safe_exit(1);
})
.amount;
.amount();
let source = SOURCE_OPT.parse(matches);
let tx_code_path = PathBuf::from(TX_BOND_WASM);
Self {
Expand Down Expand Up @@ -4311,7 +4315,7 @@ pub mod args {
println!("Could not parse bond amount: {:?}", e);
safe_exit(1);
})
.amount;
.amount();
let source = SOURCE_OPT.parse(matches);
let tx_code_path = PathBuf::from(TX_UNBOND_WASM);
Self {
Expand Down Expand Up @@ -4437,7 +4441,7 @@ pub mod args {
println!("Could not parse bond amount: {:?}", e);
safe_exit(1);
})
.amount;
.amount();
let tx_code_path = PathBuf::from(TX_REDELEGATE_WASM);
Self {
tx,
Expand Down
24 changes: 12 additions & 12 deletions apps/src/lib/config/genesis.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,10 +411,10 @@ pub fn make_dev_genesis(
StringEncoded {
raw: account_keypair.ref_to(),
},
token::DenominatedAmount {
amount: token::Amount::native_whole(200_000),
denom: NATIVE_MAX_DECIMAL_PLACES.into(),
},
token::DenominatedAmount::new(
token::Amount::native_whole(200_000),
NATIVE_MAX_DECIMAL_PLACES.into(),
),
);
}
// transfer funds from implicit key to validator
Expand All @@ -425,21 +425,21 @@ pub fn make_dev_genesis(
raw: account_keypair.ref_to(),
},
target: alias.clone(),
amount: token::DenominatedAmount {
amount: token::Amount::native_whole(200_000),
denom: NATIVE_MAX_DECIMAL_PLACES.into(),
},
amount: token::DenominatedAmount::new(
token::Amount::native_whole(200_000),
NATIVE_MAX_DECIMAL_PLACES.into(),
),
})
}
// self bond
if let Some(bonds) = genesis.transactions.bond.as_mut() {
bonds.push(transactions::BondTx {
source: transactions::AliasOrPk::Alias(alias.clone()),
validator: alias,
amount: token::DenominatedAmount {
amount: token::Amount::native_whole(100_000),
denom: NATIVE_MAX_DECIMAL_PLACES.into(),
},
amount: token::DenominatedAmount::new(
token::Amount::native_whole(100_000),
NATIVE_MAX_DECIMAL_PLACES.into(),
),
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion apps/src/lib/config/genesis/chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl Finalized {
.map(|(token, amt)| {
(
self.tokens.token.get(token).cloned().unwrap().address,
amt.amount,
amt.amount(),
)
})
.collect(),
Expand Down
6 changes: 3 additions & 3 deletions apps/src/lib/config/genesis/templates.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ pub struct EthBridgeParams {
impl TokenBalances {
pub fn get(&self, pk: common::PublicKey) -> Option<token::Amount> {
let pk = StringEncoded { raw: pk };
self.0.get(&pk).map(|amt| amt.amount)
self.0.get(&pk).map(|amt| amt.amount())
}
}
#[derive(
Expand Down Expand Up @@ -906,7 +906,7 @@ pub fn validate_balances(
let sum = next.0.values().try_fold(
token::Amount::default(),
|acc, amount| {
let res = acc.checked_add(amount.amount);
let res = acc.checked_add(amount.amount());
if res.as_ref().is_none() {
is_valid = false;
eprintln!(
Expand Down Expand Up @@ -997,7 +997,7 @@ mod tests {
.0
.get(&StringEncoded { raw: pk })
.unwrap()
.amount
.amount()
);
}
}
59 changes: 48 additions & 11 deletions apps/src/lib/config/genesis/transactions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ pub fn init_validator(
validator_wallet,
)]);

let transfer = if transfer_from_source_amount.amount.is_zero() {
let transfer = if transfer_from_source_amount.amount().is_zero() {
None
} else {
let unsigned_transfer_tx = TransferTx {
Expand All @@ -181,7 +181,7 @@ pub fn init_validator(
Some(vec![transfer_tx])
};

let bond = if self_bond_amount.amount.is_zero() {
let bond = if self_bond_amount.amount().is_zero() {
None
} else {
let unsigned_bond_tx = BondTx {
Expand Down Expand Up @@ -513,7 +513,7 @@ impl Transactions<Validated> {
BTreeMap::new();
for tx in txs {
let entry = stakes.entry(&tx.validator).or_default();
*entry += tx.amount.amount;
*entry += tx.amount.amount();
}

stakes.into_iter().any(|(_validator, stake)| {
Expand Down Expand Up @@ -1129,8 +1129,19 @@ fn validate_bond(
balances.pks.0.remove(source);
}
}
} else if let Some(new_balance) =
balance.checked_sub(*amount)
{
*balance = new_balance;
} else {
balance.amount -= amount.amount;
eprintln!(
"Invalid bond tx. Amount {} should have the \
denomination {:?}. Got {:?}.",
amount,
balance.denom(),
amount.denom(),
);
is_valid = false;
}
}
}
Expand Down Expand Up @@ -1370,7 +1381,7 @@ pub fn validate_transfer(
match balances.get_mut(token) {
Some(balances) => match balances.pks.0.get_mut(source) {
Some(balance) => {
if balance.amount < amount.amount {
if *balance < *amount {
eprintln!(
"Invalid transfer tx. Source {source} doesn't have \
enough balance of token \"{token}\" to transfer {}. \
Expand All @@ -1380,21 +1391,47 @@ pub fn validate_transfer(
is_valid = false;
} else {
// Deduct the amount from source
if amount.amount == balance.amount {
if amount == balance {
balances.pks.0.remove(source);
} else if let Some(new_balance) =
balance.checked_sub(*amount)
{
*balance = new_balance;
} else {
balance.amount -= amount.amount;
eprintln!(
"Invalid bond tx. Amount {} should have the \
denomination {:?}. Got {:?}.",
amount,
balance.denom(),
amount.denom(),
);
is_valid = false;
}

// Add the amount to target
let target_balance = balances
.aliases
.entry(target.clone())
.or_insert_with(|| DenominatedAmount {
amount: token::Amount::zero(),
denom: amount.denom,
.or_insert_with(|| {
DenominatedAmount::new(
token::Amount::zero(),
amount.denom(),
)
});
target_balance.amount += amount.amount;
if let Some(new_balance) =
target_balance.checked_add(*amount)
{
*target_balance = new_balance;
} else {
eprintln!(
"Invalid bond tx. Amount {} should have the \
denomination {:?}. Got {:?}.",
amount,
target_balance.denom(),
amount.denom(),
);
is_valid = false;
}
}
}
None => {
Expand Down
6 changes: 3 additions & 3 deletions apps/src/lib/node/ledger/shell/finalize_block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3451,7 +3451,7 @@ mod test_finalize_block {
let fee_amount =
wrapper.header().wrapper().unwrap().get_tx_fee().unwrap();
let fee_amount = fee_amount
.apply_precision(
.to_amount(
&wrapper.header().wrapper().unwrap().fee.token,
&shell.wl_storage,
)
Expand Down Expand Up @@ -3493,7 +3493,7 @@ mod test_finalize_block {
.unwrap();
assert_eq!(
new_proposer_balance,
proposer_balance.checked_add(fee_amount.amount).unwrap()
proposer_balance.checked_add(fee_amount).unwrap()
);

let new_signer_balance = storage_api::token::read_balance(
Expand All @@ -3504,7 +3504,7 @@ mod test_finalize_block {
.unwrap();
assert_eq!(
new_signer_balance,
signer_balance.checked_sub(fee_amount.amount).unwrap()
signer_balance.checked_sub(fee_amount).unwrap()
)
}

Expand Down
8 changes: 4 additions & 4 deletions apps/src/lib/node/ledger/shell/init_chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,10 @@ where
&mut self.wl_storage,
token_address,
&owner,
balance.amount,
balance.amount(),
)
.expect("Couldn't credit initial balance");
total_token_balance += balance.amount;
total_token_balance += balance.amount();
}
// Write the total amount of tokens for the ratio
self.wl_storage
Expand Down Expand Up @@ -530,7 +530,7 @@ where
token,
&source,
&target,
amount.amount,
amount.amount(),
) {
tracing::warn!(
"Genesis token transfer tx failed with: {err}. \
Expand Down Expand Up @@ -592,7 +592,7 @@ where
&mut self.wl_storage,
Some(&source),
validator,
amount.amount,
amount.amount(),
current_epoch,
Some(0),
) {
Expand Down
12 changes: 6 additions & 6 deletions apps/src/lib/node/ledger/shell/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1403,10 +1403,10 @@ where
match wrapper
.fee
.amount_per_gas_unit
.apply_precision(&wrapper.fee.token, &self.wl_storage)
.to_amount(&wrapper.fee.token, &self.wl_storage)
{
Ok(amount_per_gas_unit)
if amount_per_gas_unit.amount < minimum_gas_price =>
if amount_per_gas_unit < minimum_gas_price =>
{
// The fees do not match the minimum required
return Err(Error::TxApply(protocol::Error::FeeError(
Expand Down Expand Up @@ -2872,10 +2872,10 @@ mod shell_tests {
let mut wrapper =
Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new(
Fee {
amount_per_gas_unit: DenominatedAmount {
amount: 100.into(),
denom: apfel_denom,
},
amount_per_gas_unit: DenominatedAmount::new(
100.into(),
apfel_denom,
),
token: address::apfel(),
},
crate::wallet::defaults::albert_keypair().ref_to(),
Expand Down
16 changes: 8 additions & 8 deletions apps/src/lib/node/ledger/shell/prepare_proposal.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1146,10 +1146,10 @@ mod test_prepare_proposal {

let wrapper = WrapperTx::new(
Fee {
amount_per_gas_unit: DenominatedAmount {
amount: 100.into(),
denom: btc_denom,
},
amount_per_gas_unit: DenominatedAmount::new(
100.into(),
btc_denom,
),
token: address::btc(),
},
crate::wallet::defaults::albert_keypair().ref_to(),
Expand Down Expand Up @@ -1194,10 +1194,10 @@ mod test_prepare_proposal {

let wrapper = WrapperTx::new(
Fee {
amount_per_gas_unit: DenominatedAmount {
amount: 100.into(),
denom: apfel_denom,
},
amount_per_gas_unit: DenominatedAmount::new(
100.into(),
apfel_denom,
),
token: address::apfel(),
},
crate::wallet::defaults::albert_keypair().ref_to(),
Expand Down
8 changes: 4 additions & 4 deletions apps/src/lib/node/ledger/shell/process_proposal.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1856,10 +1856,10 @@ mod test_process_proposal {
let mut wrapper =
Tx::from_type(TxType::Wrapper(Box::new(WrapperTx::new(
Fee {
amount_per_gas_unit: DenominatedAmount {
amount: 100.into(),
denom: apfel_denom,
},
amount_per_gas_unit: DenominatedAmount::new(
100.into(),
apfel_denom,
),
token: address::apfel(),
},
crate::wallet::defaults::albert_keypair().ref_to(),
Expand Down
2 changes: 1 addition & 1 deletion core/src/ledger/ibc/context/token_transfer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ where
.into(),
)
})?;
let amount = token::DenominatedAmount { amount, denom };
let amount = token::DenominatedAmount::new(amount, denom);

Ok((token, amount))
}
Expand Down
Loading

0 comments on commit 95c5f6f

Please sign in to comment.