-
Notifications
You must be signed in to change notification settings - Fork 10
feat: deterministic resource population #443
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
base: main
Are you sure you want to change the base?
Conversation
} | ||
|
||
const allResources = [] | ||
for (let i = 0; i < 100; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100 iterations takes ~800ms on my machine. I think it's good enough to feel confident that it is indeed deterministic
There was a problem hiding this 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 ensures deterministic resource population for AlgoKit when populating app call resources. It addresses non-deterministic resource ordering that was causing edge-case failures by implementing a consistent sorting algorithm for all resource types.
- Implements deterministic sorting for all resource arrays returned by
groupResponse.unnamedResourcesAccessed
- Adds comprehensive tests to verify deterministic behavior across 100 iterations
- Updates corresponding artifact files to reflect the new smart contract functionality
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
tests/example-contracts/resource-packer/resource-packer.algo.ts | Adds new methods and properties to test deterministic resource population |
tests/example-contracts/resource-packer/artifacts/ResourcePackerv9.arc32.json | Updates compiled contract artifacts with new methods and updated bytecode |
tests/example-contracts/resource-packer/artifacts/ResourcePackerv8.arc32.json | Updates bytecode to reflect changes in the contract compilation |
tests/example-contracts/resource-packer/artifacts/InnerBoxApp.arc32.json | Updates compiled bytecode for inner box application |
tests/example-contracts/resource-packer/artifacts/ExternalApp.arc32.json | Updates compiled bytecode for external application |
src/transaction/transaction.ts | Implements deterministic sorting logic for all resource types |
src/transaction/transaction.spec.ts | Adds test to verify deterministic resource ordering across multiple iterations |
docs/code/modules/index.md | Updates line numbers in documentation due to code changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
return compare(aStr, bStr) | ||
}) | ||
sortedResources.assetHoldings?.sort((a, b) => { | ||
const aStr = `${a.asset}-${b.account}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const aStr = `${a.asset}-${b.account}` | |
const aStr = `${a.asset}-${a.account}` |
This PR ensures that resource population is deterministic. It should be noted that there is no way to guarantee that the algorithm we have is the most efficient and what works best for one group may not be best for another. For now, having determinism is a good starting point to eliminate edge-case failures. In the future we could potentially add retry mechanism with different ordering.