feat: add resource population support #225
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive resource population support to the Rust SDK's transaction composer. The feature automatically detects and populates account, app, asset, and box references required by application call transactions to prevent simulation and execution failures. The implementation includes both transaction-level and group-level resource population, with configurable enable/disable options.
Key changes:
- Added resource population functionality to transaction composer with automatic detection via simulation
- Added comprehensive test coverage for all resource types and cross-product scenarios
- Updated inner fee coverage tests to work with resource population and improved error message consistency
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/algokit_utils/tests/transactions/composer/resource_population.rs | Complete test suite for resource population feature covering accounts, apps, assets, boxes, and error scenarios |
| crates/algokit_utils/tests/transactions/composer/mod.rs | Module registration for new resource population tests |
| crates/algokit_utils/tests/transactions/composer/inner_fee_coverage.rs | Updated tests to disable resource population and improved error messages |
| crates/algokit_utils/src/transactions/composer.rs | Core resource population implementation with group-level and transaction-level logic |
| crates/algokit_transact/src/address.rs | Added utility method to compute addresses from app IDs |
| Multiple algod client model files | Corrected serialization attributes for proper msgpack encoding |
crates/algokit_utils/tests/transactions/composer/resource_population.rs
Outdated
Show resolved
Hide resolved
neilcampbell
force-pushed
the
feat/resource-population
branch
4 times, most recently
from
e4e68ff to
7024800
Compare
- Remove duplicate get_application_address function from utils.rs - Replace all usages with Address::from_app_id method from PR #225 - Update imports and function calls across codebase - Add test for Address::from_app_id method - Maintain backward compatibility for functionality Addresses PR comment by neilcampbell about using Address trait approach
…plementation - Update from_app_id to match exact implementation in PR #225 - Use hash() utility function instead of direct Sha512_256 usage - Build Vec then hash instead of incremental hashing - Remove unused Sha512_256 import and sha2 dependency - Ensures no merge conflicts when PR #225 is merged - Maintains identical application address generation logic Addresses PR review comment about using Neil's Address trait implementation
joe-p
left a comment
joe-p
left a comment
There was a problem hiding this comment.
Don't feel strong about it, but option<bool> just feels weird to me. I feel like enum would be the best option. Otherwise LGTM
neilcampbell
force-pushed
the
feat/resource-population
branch
from
a222900 to
6614c04
Compare
neilcampbell
force-pushed
the
feat/resource-population
branch
from
6614c04 to
aa35b01
Compare
joe-p
left a comment
joe-p
left a comment
There was a problem hiding this comment.
LGTM. Don't have a strong opinion on ownership/clones. I think it's good to just merge whatever you best see fit and we can optimize down the road if needed (one way or the other)
|
@joe-p Yeah I did that ownership change, it made sense and removed a number of clones. |
|
🎉 This PR is included in version 1.0.0-alpha.57 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0-alpha.49 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Add resources population support.