Skip to content

Conversation

PatrickDinh
Copy link
Collaborator

No description provided.

@PatrickDinh PatrickDinh marked this pull request as ready for review August 18, 2025 05:46
@Copilot Copilot AI review requested due to automatic review settings August 18, 2025 05:46
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds validation logic to asset transfer and asset freeze transactions to ensure asset IDs are not zero. The changes implement the Validate trait for both transaction types and integrate validation into their respective builders.

  • Implements validation logic that rejects asset IDs with value 0
  • Updates builder patterns to perform validation before creating transactions
  • Adds comprehensive test coverage for both valid and invalid scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
crates/algokit_transact/src/transactions/asset_transfer.rs Adds validation for AssetTransferTransactionFields to reject zero asset IDs and integrates validation into the builder
crates/algokit_transact/src/transactions/asset_freeze.rs Adds validation for AssetFreezeTransactionFields to reject zero asset IDs and integrates validation into the builder
Comments suppressed due to low confidence (1)

crates/algokit_transact/src/transactions/asset_transfer.rs:88

  • The test helper function create_test_address() is duplicated between asset_transfer.rs and asset_freeze.rs. Consider moving this to a shared test utilities module to reduce code duplication.
        Ok(Transaction::AssetTransfer(d))

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@PatrickDinh PatrickDinh merged commit 45bcce5 into main Aug 19, 2025
16 of 17 checks passed
@PatrickDinh PatrickDinh deleted the chore/validate-trait branch August 19, 2025 01:14
@engineering-ci
Copy link
Contributor

🎉 This PR is included in version 1.0.0-alpha.54 🎉

The release is available on:

Your semantic-release bot 📦🚀

@engineering-ci
Copy link
Contributor

🎉 This PR is included in version 1.0.0-alpha.46 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants