Skip to content

Commit 45bcce5

Browse files
PatrickDinhPatrickDinh
authored
Merge pull request #231 from algorandfoundation/chore/validate-trait
Add validations to asset config and asset freeze txns
2 parents d45e5ed + 7b3cb8b commit 45bcce5

File tree

4 files changed

+206
-44
lines changed

4 files changed

+206
-44
lines changed

‎crates/algokit_transact/src/transactions/asset_config.rs‎

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -479,14 +479,7 @@ impl Validate for AssetConfigTransactionFields {
479479
#[cfg(test)]
480480
mod tests {
481481
use super::*;
482-
use crate::test_utils::TransactionHeaderMother;
483-
484-
fn create_test_address() -> Address {
485-
// Use a valid Algorand address for testing
486-
"JB3K6HTAXODO4THESLNYTSG6GQUFNEVIQG7A6ZYVDACR6WA3ZF52TKU5NA"
487-
.parse()
488-
.unwrap()
489-
}
482+
use crate::test_utils::{AccountMother, TransactionHeaderMother};
490483

491484
#[test]
492485
fn test_validate_asset_creation_multiple_errors() {
@@ -503,7 +496,7 @@ mod tests {
503496
unit_name: Some("VERYLONGUNITNAME".to_string()), // More than 8 bytes - ERROR 4
504497
url: Some(long_url), // More than 96 bytes - ERROR 5
505498
metadata_hash: None,
506-
manager: Some(create_test_address()),
499+
manager: Some(AccountMother::neil().address()),
507500
reserve: None,
508501
freeze: None,
509502
clawback: None,
@@ -535,10 +528,10 @@ mod tests {
535528
unit_name: Some("TA".to_string()),
536529
url: Some("https://example.com".to_string()),
537530
metadata_hash: Some([1; 32]),
538-
manager: Some(create_test_address()),
539-
reserve: Some(create_test_address()),
540-
freeze: Some(create_test_address()),
541-
clawback: Some(create_test_address()),
531+
manager: Some(AccountMother::neil().address()),
532+
reserve: Some(AccountMother::neil().address()),
533+
freeze: Some(AccountMother::neil().address()),
534+
clawback: Some(AccountMother::neil().address()),
542535
};
543536

544537
let result = asset_config.validate();
@@ -551,18 +544,18 @@ mod tests {
551544
fn test_validate_valid_asset_reconfigure() {
552545
let asset_config = AssetConfigTransactionFields {
553546
header: TransactionHeaderMother::example().build().unwrap(),
554-
asset_id: 123, // Existing asset
555-
total: None, // Can't modify
556-
decimals: None, // Can't modify
557-
default_frozen: None, // Can't modify
558-
asset_name: None, // Can't modify
559-
unit_name: None, // Can't modify
560-
url: None, // Can't modify
561-
metadata_hash: None, // Can't modify
562-
manager: Some(create_test_address()), // Can modify
563-
reserve: Some(create_test_address()), // Can modify
564-
freeze: Some(create_test_address()), // Can modify
565-
clawback: Some(create_test_address()), // Can modify
547+
asset_id: 123, // Existing asset
548+
total: None, // Can't modify
549+
decimals: None, // Can't modify
550+
default_frozen: None, // Can't modify
551+
asset_name: None, // Can't modify
552+
unit_name: None, // Can't modify
553+
url: None, // Can't modify
554+
metadata_hash: None, // Can't modify
555+
manager: Some(AccountMother::neil().address()), // Can modify
556+
reserve: Some(AccountMother::neil().address()), // Can modify
557+
freeze: Some(AccountMother::neil().address()), // Can modify
558+
clawback: Some(AccountMother::neil().address()), // Can modify
566559
};
567560

568561
let result = asset_config.validate();
@@ -595,15 +588,15 @@ mod tests {
595588
fn test_validate_asset_reconfigure_multiple_immutable_field_errors() {
596589
let asset_config = AssetConfigTransactionFields {
597590
header: TransactionHeaderMother::example().build().unwrap(),
598-
asset_id: 123, // Existing asset
599-
total: Some(2000), // Can't modify total - ERROR 1
600-
decimals: Some(3), // Can't modify decimals - ERROR 2
601-
default_frozen: Some(true), // Can't modify default_frozen - ERROR 3
602-
asset_name: Some("NewName".to_string()), // Can't modify asset_name - ERROR 4
603-
unit_name: Some("NEW".to_string()), // Can't modify unit_name - ERROR 5
604-
url: Some("https://newurl.com".to_string()), // Can't modify url - ERROR 6
605-
metadata_hash: Some([2; 32]), // Can't modify metadata_hash - ERROR 7
606-
manager: Some(create_test_address()), // This is allowed
591+
asset_id: 123, // Existing asset
592+
total: Some(2000), // Can't modify total - ERROR 1
593+
decimals: Some(3), // Can't modify decimals - ERROR 2
594+
default_frozen: Some(true), // Can't modify default_frozen - ERROR 3
595+
asset_name: Some("NewName".to_string()), // Can't modify asset_name - ERROR 4
596+
unit_name: Some("NEW".to_string()), // Can't modify unit_name - ERROR 5
597+
url: Some("https://newurl.com".to_string()), // Can't modify url - ERROR 6
598+
metadata_hash: Some([2; 32]), // Can't modify metadata_hash - ERROR 7
599+
manager: Some(AccountMother::neil().address()), // This is allowed
607600
reserve: None,
608601
freeze: None,
609602
clawback: None,

‎crates/algokit_transact/src/transactions/asset_freeze.rs‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
66
use crate::Transaction;
77
use crate::address::Address;
8+
use crate::traits::Validate;
89
use crate::transactions::common::TransactionHeader;
910
use crate::utils::{is_zero, is_zero_addr};
1011
use derive_builder::Builder;
@@ -53,6 +54,90 @@ pub struct AssetFreezeTransactionFields {
5354

5455
impl AssetFreezeTransactionBuilder {
5556
pub fn build(&self) -> Result<Transaction, AssetFreezeTransactionBuilderError> {
56-
self.build_fields().map(Transaction::AssetFreeze)
57+
let d = self.build_fields()?;
58+
d.validate().map_err(|errors| {
59+
AssetFreezeTransactionBuilderError::ValidationError(format!(
60+
"Asset freeze validation failed: {}",
61+
errors.join("\n")
62+
))
63+
})?;
64+
Ok(Transaction::AssetFreeze(d))
65+
}
66+
}
67+
68+
impl Validate for AssetFreezeTransactionFields {
69+
fn validate(&self) -> Result<(), Vec<String>> {
70+
let mut errors = Vec::new();
71+
72+
if self.asset_id == 0 {
73+
errors.push("Asset ID must not be 0".to_string());
74+
}
75+
76+
match errors.is_empty() {
77+
true => Ok(()),
78+
false => Err(errors),
79+
}
80+
}
81+
}
82+
83+
#[cfg(test)]
84+
mod tests {
85+
use super::*;
86+
use crate::test_utils::{AccountMother, TransactionHeaderMother};
87+
88+
#[test]
89+
fn test_validate_asset_freeze_zero_asset_id() {
90+
let asset_freeze = AssetFreezeTransactionFields {
91+
header: TransactionHeaderMother::example().build().unwrap(),
92+
asset_id: 0, // Invalid asset ID
93+
freeze_target: AccountMother::neil().address(),
94+
frozen: true,
95+
};
96+
97+
let result = asset_freeze.validate();
98+
assert!(result.is_err());
99+
let errors = result.unwrap_err();
100+
assert_eq!(errors.len(), 1);
101+
assert_eq!(errors[0], "Asset ID must not be 0");
102+
}
103+
104+
#[test]
105+
fn test_validate_valid_asset_freeze() {
106+
let asset_freeze = AssetFreezeTransactionFields {
107+
header: TransactionHeaderMother::example().build().unwrap(),
108+
asset_id: 123, // Valid asset ID
109+
freeze_target: AccountMother::neil().address(),
110+
frozen: true,
111+
};
112+
113+
let result = asset_freeze.validate();
114+
assert!(result.is_ok());
115+
}
116+
117+
#[test]
118+
fn test_build_with_invalid_asset_id() {
119+
let result = AssetFreezeTransactionBuilder::default()
120+
.header(TransactionHeaderMother::example().build().unwrap())
121+
.asset_id(0) // Invalid asset ID
122+
.freeze_target(AccountMother::neil().address())
123+
.frozen(true)
124+
.build();
125+
126+
assert!(result.is_err());
127+
let error_message = result.unwrap_err().to_string();
128+
assert!(error_message.contains("Asset freeze validation failed"));
129+
assert!(error_message.contains("Asset ID must not be 0"));
130+
}
131+
132+
#[test]
133+
fn test_build_with_valid_asset_id() {
134+
let result = AssetFreezeTransactionBuilder::default()
135+
.header(TransactionHeaderMother::example().build().unwrap())
136+
.asset_id(123) // Valid asset ID
137+
.freeze_target(AccountMother::neil().address())
138+
.frozen(true)
139+
.build();
140+
141+
assert!(result.is_ok());
57142
}
58143
}

‎crates/algokit_transact/src/transactions/asset_transfer.rs‎

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
//!
33
//! This module provides functionality for creating and managing asset transfer transactions.
44
5+
use crate::traits::Validate;
56
use crate::transactions::common::TransactionHeader;
67
use crate::utils::{is_zero, is_zero_addr, is_zero_addr_opt};
78
use crate::{Address, Transaction};
@@ -77,6 +78,94 @@ pub struct AssetTransferTransactionFields {
7778

7879
impl AssetTransferTransactionBuilder {
7980
pub fn build(&self) -> Result<Transaction, AssetTransferTransactionBuilderError> {
80-
self.build_fields().map(Transaction::AssetTransfer)
81+
let d = self.build_fields()?;
82+
d.validate().map_err(|errors| {
83+
AssetTransferTransactionBuilderError::ValidationError(format!(
84+
"Asset transfer validation failed: {}",
85+
errors.join("\n")
86+
))
87+
})?;
88+
Ok(Transaction::AssetTransfer(d))
89+
}
90+
}
91+
92+
impl Validate for AssetTransferTransactionFields {
93+
fn validate(&self) -> Result<(), Vec<String>> {
94+
let mut errors = Vec::new();
95+
96+
if self.asset_id == 0 {
97+
errors.push("Asset ID must not be 0".to_string());
98+
}
99+
100+
match errors.is_empty() {
101+
true => Ok(()),
102+
false => Err(errors),
103+
}
104+
}
105+
}
106+
107+
#[cfg(test)]
108+
mod tests {
109+
use super::*;
110+
use crate::test_utils::{AccountMother, TransactionHeaderMother};
111+
112+
#[test]
113+
fn test_validate_asset_transfer_zero_asset_id() {
114+
let asset_transfer = AssetTransferTransactionFields {
115+
header: TransactionHeaderMother::example().build().unwrap(),
116+
asset_id: 0, // Invalid asset ID
117+
amount: 1000,
118+
receiver: AccountMother::neil().address(),
119+
asset_sender: None,
120+
close_remainder_to: None,
121+
};
122+
123+
let result = asset_transfer.validate();
124+
assert!(result.is_err());
125+
let errors = result.unwrap_err();
126+
assert_eq!(errors.len(), 1);
127+
assert_eq!(errors[0], "Asset ID must not be 0");
128+
}
129+
130+
#[test]
131+
fn test_validate_valid_asset_transfer() {
132+
let asset_transfer = AssetTransferTransactionFields {
133+
header: TransactionHeaderMother::example().build().unwrap(),
134+
asset_id: 123, // Valid asset ID
135+
amount: 1000,
136+
receiver: AccountMother::neil().address(),
137+
asset_sender: None,
138+
close_remainder_to: None,
139+
};
140+
141+
let result = asset_transfer.validate();
142+
assert!(result.is_ok());
143+
}
144+
145+
#[test]
146+
fn test_build_with_invalid_asset_id() {
147+
let result = AssetTransferTransactionBuilder::default()
148+
.header(TransactionHeaderMother::example().build().unwrap())
149+
.asset_id(0) // Invalid asset ID
150+
.amount(1000)
151+
.receiver(AccountMother::neil().address())
152+
.build();
153+
154+
assert!(result.is_err());
155+
let error_message = result.unwrap_err().to_string();
156+
assert!(error_message.contains("Asset transfer validation failed"));
157+
assert!(error_message.contains("Asset ID must not be 0"));
158+
}
159+
160+
#[test]
161+
fn test_build_with_valid_asset_id() {
162+
let result = AssetTransferTransactionBuilder::default()
163+
.header(TransactionHeaderMother::example().build().unwrap())
164+
.asset_id(123) // Valid asset ID
165+
.amount(1000)
166+
.receiver(AccountMother::neil().address())
167+
.build();
168+
169+
assert!(result.is_ok());
81170
}
82171
}

‎crates/algokit_utils/src/transactions/asset_transfer.rs‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,18 +103,13 @@ pub fn build_asset_clawback(
103103
#[cfg(test)]
104104
mod tests {
105105
use super::*;
106-
use algokit_transact::{Address, TransactionHeader};
107-
use std::str::FromStr;
106+
use algokit_transact::{TransactionHeader, test_utils::AccountMother};
108107

109108
#[test]
110109
fn test_asset_opt_out_with_optional_close_remainder_to() {
111110
// Use valid test addresses
112-
let sender =
113-
Address::from_str("JB3K6HTAXODO4THESLNYTSG6GQUFNEVIQG7A6ZYVDACR6WA3ZF52TKU5NA")
114-
.unwrap();
115-
let creator =
116-
Address::from_str("JB3K6HTAXODO4THESLNYTSG6GQUFNEVIQG7A6ZYVDACR6WA3ZF52TKU5NA")
117-
.unwrap();
111+
let sender = AccountMother::neil().address();
112+
let creator = AccountMother::neil().address();
118113

119114
// Test with Some(creator) - explicit close_remainder_to
120115
let params_with_creator = AssetOptOutParams {

0 commit comments

Comments
 (0)