Skip to content

Commit

Permalink
Changes some MASP types to be unsigned
Browse files Browse the repository at this point in the history
  • Loading branch information
@grarco
grarco committed Sep 6, 2024
1 parent 6d1b9da commit ffd2f35
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 53 deletions.
8 changes: 3 additions & 5 deletions crates/core/src/token.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,16 +232,14 @@ impl Amount {
Self { raw: Uint(raw) }
}

/// Given a i128 and [`MaspDigitPos`], construct the corresponding
/// Given a u128 and [`MaspDigitPos`], construct the corresponding
/// amount.
pub fn from_masp_denominated_i128(
val: i128,
pub fn from_masp_denominated_u128(
val: u128,
denom: MaspDigitPos,
) -> Option<Self> {
#[allow(clippy::cast_sign_loss)]
#[allow(clippy::cast_possible_truncation)]
let lo = val as u64;
#[allow(clippy::cast_sign_loss)]
let hi = (val >> 64) as u64;
let lo_pos = denom as usize;
let hi_pos = lo_pos.checked_add(1)?;
Expand Down
22 changes: 9 additions & 13 deletions crates/shielded_token/src/masp.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use masp_primitives::sapling::keys::FullViewingKey;
use masp_primitives::sapling::{Diversifier, Node, ViewingKey};
use masp_primitives::transaction::builder::{self, *};
use masp_primitives::transaction::components::sapling::builder::SaplingMetadata;
use masp_primitives::transaction::components::{I128Sum, ValueSum};
use masp_primitives::transaction::components::{I128Sum, U128Sum, ValueSum};
use masp_primitives::transaction::Transaction;
use masp_primitives::zip32::{
ExtendedFullViewingKey, ExtendedSpendingKey as MaspExtendedSpendingKey,
Expand Down Expand Up @@ -131,7 +131,7 @@ pub struct MaspTxReorderedData {
/// Data about the unspent amounts for any given shielded source coming from the
/// spent notes in their posses that have been added to the builder. Can be used
/// to either pay fees or to return a change
pub type Changes = HashMap<namada_core::masp::ExtendedSpendingKey, I128Sum>;
pub type Changes = HashMap<namada_core::masp::ExtendedSpendingKey, U128Sum>;

/// Shielded pool data for a token
#[allow(missing_docs)]
Expand Down Expand Up @@ -253,7 +253,7 @@ pub fn is_amount_required(
dest: I128Sum,
normed_delta: I128Sum,
opt_delta: Option<I128Sum>,
) -> Option<I128Sum> {
) -> Option<U128Sum> {
let mut changes = None;
let gap = dest.clone() - src;

Expand All @@ -262,19 +262,16 @@ pub fn is_amount_required(
let signed_change_amt =
checked!(normed_delta[asset_type] - *value).unwrap_or_default();
let unsigned_change_amt = if signed_change_amt > 0 {
signed_change_amt
signed_change_amt as u128
} else {
// Even if there's no change we still need to set the return
// value of this function to be Some so that the caller sees
// that this note should be used
0
};

let change_amt = I128Sum::from_nonnegative(
asset_type.to_owned(),
unsigned_change_amt,
)
.expect("Change is guaranteed to be non-negative");
let change_amt =
U128Sum::from_pair(asset_type.to_owned(), unsigned_change_amt);
changes = changes
.map(|prev| prev + change_amt.clone())
.or(Some(change_amt));
Expand All @@ -289,11 +286,10 @@ pub fn is_amount_required(
changes = changes.map(|mut chngs| {
for (delta_asset_type, delta_amt) in delta.components() {
if !dest.asset_types().contains(delta_asset_type) {
let rmng = I128Sum::from_nonnegative(
let rmng = U128Sum::from_pair(
delta_asset_type.to_owned(),
*delta_amt,
)
.expect("Change is guaranteed to be non-negative");
*delta_amt as u128,
);
chngs += rmng;
}
}
Expand Down
73 changes: 38 additions & 35 deletions crates/shielded_token/src/masp/shielded_wallet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use masp_primitives::sapling::{
use masp_primitives::transaction::builder::Builder;
use masp_primitives::transaction::components::sapling::builder::RngBuildParams;
use masp_primitives::transaction::components::{
I128Sum, TxOut, U64Sum, ValueSum,
I128Sum, TxOut, U128Sum, U64Sum, ValueSum,
};
use masp_primitives::transaction::fees::fixed::FeeRule;
use masp_primitives::transaction::{builder, Transaction};
Expand Down Expand Up @@ -1502,16 +1502,10 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
(asset_types, amount)
};

let mut fees = I128Sum::zero();
// Convert the shortfall into a I128Sum
let mut fees = U128Sum::zero();
// Convert the shortfall into a U128Sum
for (asset_type, val) in asset_types.iter().zip(raw_amount) {
fees += I128Sum::from_nonnegative(*asset_type, val.into())
.map_err(|()| {
TransferErr::General(
"Fee amount is expected expected to be non-negative"
.to_string(),
)
})?;
fees += U128Sum::from_pair(*asset_type, val.into());
}

// 1. Try to use the change to pay fees
Expand All @@ -1526,22 +1520,24 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
{
// Get the minimum between the available change and
// the due fee
let output_amt = I128Sum::from_nonnegative(
let output_amt = U128Sum::from_pair(
asset_type.to_owned(),
*change.min(fee_amt),
)
.map_err(|()| {
TransferErr::General(
"Fee amount is expected to be non-negative"
.to_string(),
)
})?;
);
let denominated_output_amt = self
.convert_masp_amount_to_namada(
context.client(),
// Safe to unwrap
denoms.get(token).unwrap().to_owned(),
output_amt.clone(),
I128Sum::try_from_sum(output_amt.clone()).map_err(
|e| {
TransferErr::General(format!(
"Fee amount is expected to be \
non-negative: {}",
e
))
},
)?,
)
.await
.map_err(|e| TransferErr::General(e.to_string()))?;
Expand Down Expand Up @@ -1575,16 +1571,8 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
// Decrease the changes by the amounts used for fee payment
for (sp, temp_changes) in temp_changes.iter() {
for (asset_type, temp_change) in temp_changes.components() {
let output_amt = I128Sum::from_nonnegative(
asset_type.to_owned(),
*temp_change,
)
.map_err(|()| {
TransferErr::General(
"Fee amount is expected expected to be non-negative"
.to_string(),
)
})?;
let output_amt =
U128Sum::from_pair(asset_type.to_owned(), *temp_change);

// Entry is guaranteed to be in the map
changes.entry(*sp).and_modify(|amt| *amt -= &output_amt);
Expand All @@ -1604,7 +1592,8 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
for (asset_type, fee_amt) in fees.clone().components() {
let input_amt = I128Sum::from_nonnegative(
asset_type.to_owned(),
*fee_amt,
i128::try_from(*fee_amt)
.map_err(|e| TransferErr::General(e.to_string()))?,
)
.map_err(|()| {
TransferErr::General(
Expand Down Expand Up @@ -1669,7 +1658,12 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
.await
.map_err(|e| TransferErr::General(e.to_string()))?;

fees -= &output_amt;
fees -= U128Sum::try_from_sum(output_amt).map_err(|e| {
TransferErr::General(format!(
"Output amount is expected to be non-negative: {}",
e
))
})?;
}

if fees.is_zero() {
Expand All @@ -1679,8 +1673,15 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
}

if !fees.is_zero() {
let signed_fees = I128Sum::try_from_sum(fees).map_err(|e| {
TransferErr::General(format!(
"Fee amount cannot be casted to a signed integer: {}",
e
))
})?;

return Result::Err(TransferErr::Build {
error: builder::Error::InsufficientFunds(fees),
error: builder::Error::InsufficientFunds(signed_fees),
data: Some(MaspDataLog {
source: None,
token: token.to_owned(),
Expand Down Expand Up @@ -1806,9 +1807,11 @@ pub trait ShieldedApi<U: ShieldedUtils + MaybeSend + MaybeSync>:
let value_sum = self.decode_sum(client, amt).await;

for ((_, decoded), val) in value_sum.components() {
let positioned_amt =
Amount::from_masp_denominated_i128(*val, decoded.position)
.unwrap_or_default();
let positioned_amt = Amount::from_masp_denominated_u128(
*val as u128,
decoded.position,
)
.unwrap_or_default();
amount = checked!(amount + positioned_amt)?;
}

Expand Down

0 comments on commit ffd2f35

Please sign in to comment.