fix: ensure byte arrays are decoded as Uint8Array #406
Conversation
joe-p
force-pushed
the
fix/decoded_byte_arrays
branch
2 times, most recently
from
5fd4c1d to
992c756
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a utility to recursively convert decoded ABI byte arrays into Uint8Array and applies it across method return decoding, composer results, state getters, and ARC56 struct decoding.
- Introduce
convertAbiByteArraysinsrc/util.tsand include extensive tests. - Update all code paths (transaction composer, app manager, app client, ARC56 decoding) to apply the conversion.
- Add byte array examples and tests in
tests/example-contracts/byte_arrays.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/example-contracts/byte_arrays/... | Add example contract, configs, and artifacts for byte array tests |
| src/util.ts | Implement convertAbiByteArrays to normalize byte arrays |
| src/util.spec.ts | Add unit tests covering various ABI value conversion scenarios |
| src/types/composer.ts | Pass ABI return types into composer’s return mapping |
| src/types/app-manager.ts | Pass ABI return types into app manager response parsing |
| src/types/app-client.ts | Tighten state getter error on missing metadata |
| src/types/app-arc56.ts | Convert struct and decoded values through convertAbiByteArrays |
| src/transaction/transaction.ts | Apply byte-array conversion in transaction composer and return decoding |
| @@ -2135,13 +2135,14 @@ export class TransactionComposer { | |||
| } | |||
|
|
|||
| const transactions = atc.buildGroup().map((t) => t.txn) | |||
| const methodCalls = [...(atc['methodCalls'] as Map<number, ABIMethod>).values()] | |||
There was a problem hiding this comment.
[nitpick] Accessing the private atc['methodCalls'] map is brittle and may break if the internal API changes. Consider using a public property or capturing method calls at compose time instead.
| const methodCalls = [...(atc['methodCalls'] as Map<number, ABIMethod>).values()] | |
| const methodCalls = [...methodCalls.values()] |
Copilot uses AI. Check for mistakes.
| @@ -846,12 +854,14 @@ export const sendAtomicTransactionComposer = async function (atcSend: AtomicTran | |||
| confirmations = await Promise.all(transactionsToSend.map(async (t) => await algod.pendingTransactionInformation(t.txID()).do())) | |||
| } | |||
|
|
|||
| const methodCalls = [...(atc['methodCalls'] as Map<number, ABIMethod>).values()] | |||
There was a problem hiding this comment.
[nitpick] Similar to the composer, relying on the internal methodCalls property of the ATC is fragile. It’s better to track calls through the public API or store them when invoking the composer.
Copilot uses AI. Check for mistakes.
joe-p
force-pushed
the
fix/decoded_byte_arrays
branch
from
992c756 to
56dc9d2
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
joe-p
force-pushed
the
fix/decoded_byte_arrays
branch
from
cdefa61 to
b07f0fa
Compare
|
🎉 This PR is included in version 9.1.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR ensures all byte arrays, even nested ones, are decoded as Uint8Array. This change affects the following values
Composermethod calls (and thusAppClientmethod calls)Composersimulation resultsAppClientstate valuesAppClientstate keysWe ideally should have consistent decoding everywhere, so if there's other places that we are decoding ABI values that I've missed let me know!
Fixes #395