Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back output amount 0 check #1117

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 9 additions & 5 deletions sdk/src/types/block/output/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use packable::{
Packable,
};

use super::verify_output_amount_packable;
use crate::types::{
block::{
address::{AccountAddress, Address},
Expand All @@ -22,8 +21,9 @@ use crate::types::{
unlock_condition::{
verify_allowed_unlock_conditions, UnlockCondition, UnlockConditionFlags, UnlockConditions,
},
verify_output_amount, AccountId, ChainId, NativeToken, NativeTokens, Output, OutputBuilderAmount, OutputId,
Rent, RentStructure, StateTransitionError, StateTransitionVerifier,
verify_output_amount_min, verify_output_amount_packable, verify_output_amount_supply, AccountId, ChainId,
NativeToken, NativeTokens, Output, OutputBuilderAmount, OutputId, Rent, RentStructure,
StateTransitionError, StateTransitionVerifier,
},
protocol::ProtocolParameters,
semantic::{TransactionFailureReason, ValidationContext},
Expand Down Expand Up @@ -299,6 +299,8 @@ impl AccountOutputBuilder {
}
};

verify_output_amount_min(output.amount)?;

Ok(output)
}

Expand All @@ -310,7 +312,7 @@ impl AccountOutputBuilder {
let output = self.finish()?;

if let Some(token_supply) = params.into().token_supply() {
verify_output_amount(&output.amount, &token_supply)?;
verify_output_amount_supply(output.amount, token_supply)?;
}

Ok(output)
Expand Down Expand Up @@ -918,8 +920,9 @@ mod tests {
let sender_2 = rand_sender_feature();
let issuer_1 = rand_issuer_feature();
let issuer_2 = rand_issuer_feature();
let amount = 500_000;

let mut builder = AccountOutput::build_with_amount(0, account_id)
let mut builder = AccountOutput::build_with_amount(amount, account_id)
.add_native_token(NativeToken::new(TokenId::from(foundry_id), 1000).unwrap())
.add_unlock_condition(gov_address_1)
.add_unlock_condition(state_address_1)
Expand All @@ -929,6 +932,7 @@ mod tests {
.add_immutable_feature(issuer_2);

let output = builder.clone().finish().unwrap();
assert_eq!(output.amount(), amount);
assert_eq!(output.unlock_conditions().governor_address(), Some(&gov_address_1));
assert_eq!(
output.unlock_conditions().state_controller_address(),
Expand Down
13 changes: 8 additions & 5 deletions sdk/src/types/block/output/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use alloc::collections::BTreeSet;

use packable::Packable;

use super::verify_output_amount_packable;
use crate::types::{
block::{
address::Address,
Expand All @@ -14,8 +13,8 @@ use crate::types::{
unlock_condition::{
verify_allowed_unlock_conditions, UnlockCondition, UnlockConditionFlags, UnlockConditions,
},
verify_output_amount, NativeToken, NativeTokens, Output, OutputBuilderAmount, OutputId, Rent,
RentStructure,
verify_output_amount_min, verify_output_amount_packable, verify_output_amount_supply, NativeToken,
NativeTokens, Output, OutputBuilderAmount, OutputId, Rent, RentStructure,
},
protocol::ProtocolParameters,
semantic::{TransactionFailureReason, ValidationContext},
Expand Down Expand Up @@ -177,6 +176,8 @@ impl BasicOutputBuilder {
}
};

verify_output_amount_min(output.amount)?;

Ok(output)
}

Expand All @@ -185,7 +186,7 @@ impl BasicOutputBuilder {
let output = self.finish()?;

if let Some(token_supply) = params.into().token_supply() {
verify_output_amount(&output.amount, &token_supply)?;
verify_output_amount_supply(output.amount, token_supply)?;
}

Ok(output)
Expand Down Expand Up @@ -481,14 +482,16 @@ mod tests {
let address_2 = rand_address_unlock_condition();
let sender_1 = rand_sender_feature();
let sender_2 = rand_sender_feature();
let amount = 500_000;

let mut builder = BasicOutput::build_with_amount(0)
let mut builder = BasicOutput::build_with_amount(amount)
.add_native_token(NativeToken::new(TokenId::from(foundry_id), 1000).unwrap())
.add_unlock_condition(address_1)
.add_feature(sender_1)
.replace_feature(sender_2);

let output = builder.clone().finish().unwrap();
assert_eq!(output.amount(), amount);
assert_eq!(output.unlock_conditions().address(), Some(&address_1));
assert_eq!(output.features().sender(), Some(&sender_2));

Expand Down
11 changes: 6 additions & 5 deletions sdk/src/types/block/output/delegation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ use crate::types::{
unlock_condition::{
verify_allowed_unlock_conditions, UnlockCondition, UnlockConditionFlags, UnlockConditions,
},
verify_output_amount, Output, OutputBuilderAmount, OutputId, Rent, RentStructure,
verify_output_amount_min, verify_output_amount_packable, verify_output_amount_supply, Output,
OutputBuilderAmount, OutputId, Rent, RentStructure,
},
protocol::ProtocolParameters,
semantic::{TransactionFailureReason, ValidationContext},
Expand Down Expand Up @@ -196,6 +197,8 @@ impl DelegationOutputBuilder {
}
};

verify_output_amount_min(output.amount)?;

Ok(output)
}

Expand All @@ -207,7 +210,7 @@ impl DelegationOutputBuilder {
let output = self.finish()?;

if let Some(token_supply) = params.into().token_supply() {
verify_output_amount(&output.amount, &token_supply)?;
verify_output_amount_supply(output.amount, token_supply)?;
}

Ok(output)
Expand Down Expand Up @@ -376,9 +379,7 @@ impl Packable for DelegationOutput {
) -> Result<Self, UnpackError<Self::UnpackError, U::Error>> {
let amount = u64::unpack::<_, VERIFY>(unpacker, &()).coerce()?;

if VERIFY {
verify_output_amount(&amount, &visitor.token_supply()).map_err(UnpackError::Packable)?;
}
verify_output_amount_packable::<VERIFY>(&amount, visitor).map_err(UnpackError::Packable)?;

let delegated_amount = u64::unpack::<_, VERIFY>(unpacker, &()).coerce()?;
let delegation_id = DelegationId::unpack::<_, VERIFY>(unpacker, &()).coerce()?;
Expand Down
14 changes: 9 additions & 5 deletions sdk/src/types/block/output/foundry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use packable::{
};
use primitive_types::U256;

use super::verify_output_amount_packable;
use crate::types::{
block::{
address::{AccountAddress, Address},
Expand All @@ -21,8 +20,9 @@ use crate::types::{
unlock_condition::{
verify_allowed_unlock_conditions, UnlockCondition, UnlockConditionFlags, UnlockConditions,
},
verify_output_amount, ChainId, FoundryId, NativeToken, NativeTokens, Output, OutputBuilderAmount, OutputId,
Rent, RentStructure, StateTransitionError, StateTransitionVerifier, TokenId, TokenScheme,
verify_output_amount_min, verify_output_amount_packable, verify_output_amount_supply, ChainId, FoundryId,
NativeToken, NativeTokens, Output, OutputBuilderAmount, OutputId, Rent, RentStructure,
StateTransitionError, StateTransitionVerifier, TokenId, TokenScheme,
},
protocol::ProtocolParameters,
semantic::{TransactionFailureReason, ValidationContext},
Expand Down Expand Up @@ -238,6 +238,8 @@ impl FoundryOutputBuilder {
}
};

verify_output_amount_min(output.amount)?;

Ok(output)
}

Expand All @@ -249,7 +251,7 @@ impl FoundryOutputBuilder {
let output = self.finish()?;

if let Some(token_supply) = params.into().token_supply() {
verify_output_amount(&output.amount, &token_supply)?;
verify_output_amount_supply(output.amount, token_supply)?;
}

Ok(output)
Expand Down Expand Up @@ -766,8 +768,9 @@ mod tests {
let account_2 = ImmutableAccountAddressUnlockCondition::new(rand_account_address());
let metadata_1 = rand_metadata_feature();
let metadata_2 = rand_metadata_feature();
let amount = 500_000;

let mut builder = FoundryOutput::build_with_amount(0, 234, rand_token_scheme())
let mut builder = FoundryOutput::build_with_amount(amount, 234, rand_token_scheme())
.with_serial_number(85)
.add_native_token(NativeToken::new(TokenId::from(foundry_id), 1000).unwrap())
.with_unlock_conditions([account_1])
Expand All @@ -777,6 +780,7 @@ mod tests {
.replace_immutable_feature(metadata_1.clone());

let output = builder.clone().finish().unwrap();
assert_eq!(output.amount(), amount);
assert_eq!(output.serial_number(), 85);
assert_eq!(output.unlock_conditions().immutable_account_address(), Some(&account_1));
assert_eq!(output.features().metadata(), Some(&metadata_2));
Expand Down
21 changes: 17 additions & 4 deletions sdk/src/types/block/output/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,20 +472,33 @@ impl Rent for Output {
}
}

pub(crate) fn verify_output_amount(amount: &u64, token_supply: &u64) -> Result<(), Error> {
if *amount < Output::AMOUNT_MIN || amount > token_supply {
Err(Error::InvalidOutputAmount(*amount))
pub(crate) fn verify_output_amount_min(amount: u64) -> Result<(), Error> {
if amount < Output::AMOUNT_MIN {
Err(Error::InvalidOutputAmount(amount))
} else {
Ok(())
}
}

pub(crate) fn verify_output_amount_supply(amount: u64, token_supply: u64) -> Result<(), Error> {
if amount > token_supply {
Err(Error::InvalidOutputAmount(amount))
} else {
Ok(())
}
}

pub(crate) fn verify_output_amount(amount: u64, token_supply: u64) -> Result<(), Error> {
verify_output_amount_min(amount)?;
verify_output_amount_supply(amount, token_supply)
}

pub(crate) fn verify_output_amount_packable<const VERIFY: bool>(
amount: &u64,
protocol_parameters: &ProtocolParameters,
) -> Result<(), Error> {
if VERIFY {
verify_output_amount(amount, &protocol_parameters.token_supply())?;
verify_output_amount(*amount, protocol_parameters.token_supply())?;
}
Ok(())
}
Expand Down
14 changes: 9 additions & 5 deletions sdk/src/types/block/output/nft.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use packable::{
Packable,
};

use super::verify_output_amount_packable;
use crate::types::{
block::{
address::{Address, NftAddress},
Expand All @@ -19,8 +18,9 @@ use crate::types::{
unlock_condition::{
verify_allowed_unlock_conditions, UnlockCondition, UnlockConditionFlags, UnlockConditions,
},
verify_output_amount, ChainId, NativeToken, NativeTokens, NftId, Output, OutputBuilderAmount, OutputId,
Rent, RentStructure, StateTransitionError, StateTransitionVerifier,
verify_output_amount_min, verify_output_amount_packable, verify_output_amount_supply, ChainId, NativeToken,
NativeTokens, NftId, Output, OutputBuilderAmount, OutputId, Rent, RentStructure, StateTransitionError,
StateTransitionVerifier,
},
protocol::ProtocolParameters,
semantic::{TransactionFailureReason, ValidationContext},
Expand Down Expand Up @@ -224,6 +224,8 @@ impl NftOutputBuilder {
}
};

verify_output_amount_min(output.amount)?;

Ok(output)
}

Expand All @@ -232,7 +234,7 @@ impl NftOutputBuilder {
let output = self.finish()?;

if let Some(token_supply) = params.into().token_supply() {
verify_output_amount(&output.amount, &token_supply)?;
verify_output_amount_supply(output.amount, token_supply)?;
}

Ok(output)
Expand Down Expand Up @@ -651,8 +653,9 @@ mod tests {
let sender_2 = rand_sender_feature();
let issuer_1 = rand_issuer_feature();
let issuer_2 = rand_issuer_feature();
let amount = 500_000;

let mut builder = NftOutput::build_with_amount(0, NftId::null())
let mut builder = NftOutput::build_with_amount(amount, NftId::null())
.add_native_token(NativeToken::new(TokenId::from(foundry_id), 1000).unwrap())
.add_unlock_condition(address_1)
.add_feature(sender_1)
Expand All @@ -661,6 +664,7 @@ mod tests {
.add_immutable_feature(issuer_2);

let output = builder.clone().finish().unwrap();
assert_eq!(output.amount(), amount);
assert_eq!(output.unlock_conditions().address(), Some(&address_1));
assert_eq!(output.features().sender(), Some(&sender_2));
assert_eq!(output.immutable_features().issuer(), Some(&issuer_1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ impl StorageDepositReturnUnlockCondition {
/// Creates a new [`StorageDepositReturnUnlockCondition`].
#[inline(always)]
pub fn new(return_address: impl Into<Address>, amount: u64, token_supply: u64) -> Result<Self, Error> {
verify_amount::<true>(&amount, &token_supply)?;
verify_amount::<true>(amount, token_supply)?;

Ok(Self {
return_address: return_address.into(),
Expand All @@ -43,9 +43,9 @@ impl StorageDepositReturnUnlockCondition {
}
}

fn verify_amount<const VERIFY: bool>(amount: &u64, token_supply: &u64) -> Result<(), Error> {
fn verify_amount<const VERIFY: bool>(amount: u64, token_supply: u64) -> Result<(), Error> {
if VERIFY {
verify_output_amount(amount, token_supply).map_err(|_| Error::InvalidStorageDepositAmount(*amount))?;
verify_output_amount(amount, token_supply).map_err(|_| Error::InvalidStorageDepositAmount(amount))?;
}

Ok(())
Expand All @@ -55,7 +55,7 @@ fn verify_amount_packable<const VERIFY: bool>(
amount: &u64,
protocol_parameters: &ProtocolParameters,
) -> Result<(), Error> {
verify_amount::<VERIFY>(amount, &protocol_parameters.token_supply())
verify_amount::<VERIFY>(*amount, protocol_parameters.token_supply())
}

#[cfg(feature = "serde")]
Expand Down