Repository navigation
Add typed ERC-20 transfer encoding operation - #78
Merged
Merged
Conversation
Add a round-trip test asserting the explicit JSON shape of the ERC-20 transfer encode request and response, matching the existing wire-shape tests for the other message types. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new bus method that encodes standard ERC-20 transfer calldata inside the module using the module-owned ABI implementation, exposing request and response wire types and bumping the contract version to 1.13. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add an end-to-end case that calls EncodeErc20Transfer on the loaded module and checks the calldata against the expected ABI encoding, so the algorithm is proven to run inside the artifact rather than via host-linked crypto. The same test asserts that an invalid recipient is rejected with Web3OperationFailed, and the method is added to the expected manifest list. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Register the encode ERC20 transfer request and response types in the wire module and expose the corresponding wallet service method, bringing the dispatch table to 38 methods. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
ERC-20 transfer encoding failures now surface as InvalidInput instead of Web3OperationFailed, since malformed recipient or amount values are caller errors rather than upstream web3 faults. The end-to-end test was updated to expect the new error variant. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…o wallet-x402-multisecret
Extend the lifecycle end-to-end test's expected method list to cover the EncodeErc20Transfer RPC, keeping the assertion in sync with the module's exposed interface. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing (9)
✨ Finishing Touches
Comment |
Tiny Sweeper reviewThis pull request adds a new typed ERC-20 transfer encoding operation named `EncodeErc20Transfer` to the TinyWallet bus contract. It introduces request/response wire types, registers the method in the method-name list and contract version, implements the module-side handler delegating to `tinywallet_x402::abi::encode_erc20_transfer`, and updates unit and end-to-end tests. The contract version is bumped from (1, 12) to (1, 13). No review findings were reported across the critique, security, tests, and commits lanes; some reviewers did not produce usable responses for the module service file, the module e2e test file, and the PR description. State: Incomplete Review snapshot
Completeness: Incomplete What changedThe wire contract gains `EncodeErc20TransferRequest` and `EncodeErc20TransferResponse` structs in `crates/tinywallet-bus/src/wire/wallet.rs`, both with `deny_unknown_fields`. The method name `EncodeErc20Transfer` is added to the `names::methods` constants and inserted into the sorted `METHODS` list, and the contract version is raised to (1, 13) in `crates/tinywallet-bus/src/version/mod.rs`. The re-export block in `crates/tinywallet-bus/src/wire/mod.rs` is reformatted to include the two new types. On the module side, `crates/tinywallet-module/src/service/mod.rs` adds an async `encode_erc20_transfer` handler that maps encoding errors to `invalid_input` and appends `EncodeErc20Transfer` to the exported method list. Test updates cover the wire shape and round trip, the cumulative method count, the module manifest expectations, and a new in-artifact encoding call with an invalid-address error path. Features
Tests
FindingsNo active actionable findings. Could not review: crates/tinywallet-module/src/service/mod.rs, crates/tinywallet-module/tests/module_e2e.rs, tinysweeper/description, tinysweeper/tests Before merge
How this fits togetherflowchart LR
n0["...s_are_additive_and_use_one_typed_argument<br/>changed"]:::changed
n1["...is_sorted_complete_and_free_of_duplicates<br/>changed"]:::changed
n2["METHODS"]:::impacted
n3["derive"]:::impacted
n4["SecretMaterial"]:::impacted
n5["...dule_signs_every_chain_over_a_real_broker"]:::impacted
n6["NATIVE_WALLET_EXECUTE_PREPARED"]:::impacted
n7["...ucts_evm_transactions_inside_the_artifact"]:::impacted
n0 -->|uses| n6
n1 -->|uses| n6
n2 -->|uses| n6
n5 -->|uses| n4
n7 -->|calls| n3
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinywallet-module/src/service/mod.rs, crates/tinywallet-module/tests/module_e2e.rs, tinysweeper/description, tinysweeper/tests.
$0.0128 · 892,719 in / 66,580 out · 224,192 cached (25%) · ladder/vectors-oai3, deepseek-v4.1-flash, glm-5.3-flash · 544 embedded
critique: $0.0026 · 401,367 in / 27,883 out · 93,440 cached (23%) · deepseek-v4.1-flash
security: $0.0096 · 425,839 in / 30,105 out · 109,248 cached (26%) · deepseek-v4.1-flash, glm-5.3-flash
tests: $0.0003 · 39,436 in / 3,763 out · 15,360 cached (39%) · deepseek-v4.1-flash
description: $0.0002 · 20,496 in / 3,297 out · 6,144 cached (30%) · deepseek-v4.1-flash
tinysweeper
Bot
added
the
priority: p3
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the typed
EncodeErc20Transferbus operation so OpenHuman can keep generic HTTP transport while ERC-20 calldata encoding remains in TinyWallet’s x402 implementation. The operation only adds serialized request/response vocabulary and a module dispatch; the existing calldata encoder remains the implementation source of truth.The TinyWallet bus contract minor version advances to 1.13. Existing method names and arities remain unchanged.
Verification
cargo test -p tinywallet-bus --all-features(44 passed)cargo test --all-featurescargo +stable clippy --all-targets --all-features -- -D warningscargo +stable build --all-targets --all-featurescargo fmt --all -- --checkscripts/test-e2e.sh: compiled TinyBus artifact loaded successfully; ERC-20 encoding and invalid-input behavior passed (2 module E2E tests, 1 lifecycle E2E test).Local logs are in
.migration-validation/wallet-erc20/and are not part of the source diff.