Move wallet transaction building into the tinywallet module (-51 crates) - #5495
Conversation
Updated the vendored tinywallet dependency to a newer commit to pick up upstream fixes and improvements. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Update the pinned commit for the tinywallet vendored dependency to incorporate upstream fixes and improvements. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the tinywallet library as a vendored dependency to support wallet-related functionality in the project. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Added the tinywallet library as a vendored dependency to support wallet-related functionality in the project. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Added the tinywallet library as a new vendored dependency to support wallet-related functionality in the project. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the tinywallet vendored dependency to a newer version, incorporating upstream fixes and improvements. The wallet module now uses the updated API without any behavioural changes to the application. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test now expects the correct fee amount after fixing an off-by-one error in the fee calculation logic, ensuring the test accurately validates the intended behavior. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the module registry initializes, it now creates the module directory if it does not exist instead of failing. This ensures a clean first-run experience without requiring manual directory setup. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When a module directory does not exist, the registry now returns an empty list of modules instead of panicking. This allows the system to continue operating normally when modules have not yet been installed or have been removed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add two tests that guard against shipping the tinywallet module without published artifacts. The first prints a reminder when assets are missing, and the second asserts that an artifact-less module produces no platform candidates, ensuring the empty list fails safe rather than falling back to an unverified download. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `tinywallet` dependency is now loaded without the `tx` or `client` features, which were the only gates pulling in the `bitcoin` crate and its native secp256k1 build. Transaction building has moved into the loaded `tinywallet` module, so the remaining features—address validation, key derivation, the transport seam, EIP-712 hashing, and ERC-20 calldata—require no chain library. The `k256` crate is added directly for secp256k1 signing over the digests the wallet module returns; it is already in the dependency graph via `coins-bip32`, so naming it explicitly costs nothing and allows the `bitcoin` crate to be removed. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The Cargo.lock file is updated to reflect changes in the tinywallet crate's dependencies, adding k256, bech32, coins-bip32, and ripemd while removing the direct bitcoin dependency, likely to support new cryptographic operations and address encoding. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The bitcoin crate is added as a dependency of the web3 feature because the four chain modules and x402 have not yet been migrated onto the loaded wallet module. Removing these dependencies will be the final step of the port, completing the planned 51-crate reduction. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test for a module without artifacts now checks resolution against real host coordinates instead of using an abstract candidate lookup, and it also verifies that a different module with assets still resolves on the same hosts. This rules out a fallback that would incorrectly assign one module's artifact to another. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The tinywallet module record and its associated tests have been removed because the module cannot be registered until its v0.2.0 release provides verifiable artifact digests. The empty asset list that previously served as a placeholder was removed along with the record, as the module will be added back with the actual release artifacts when they become available. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat several multi-line function calls and match arms in the wallet module to fit on single lines, and remove a trailing blank line in the registry test module. These are purely cosmetic changes with no effect on behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replaced the manual modulo check with the more idiomatic `is_multiple_of` method when validating that a hex string has an even number of characters. This improves code readability without changing the behaviour. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the ABI encoding implementation that was previously omitted from the wallet module, enabling proper serialization of function calls and event data when interacting with the smart contract. This resolves runtime errors that occurred when attempting to encode transaction parameters for the wallet contract. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace the `hex_to_u256` and `u256_to_hex` functions with `hex_to_u128` and `u128_to_hex` respectively, removing the dependency on `ethers_core::types::U256`. The wallet never needs to handle values exceeding 2^128, so using a native u128 simplifies the code and eliminates an unnecessary dependency. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…tion Replace the direct use of ethers for transaction construction and signing with a call to the wallet module, which now handles encoding and returns a digest for this process to sign. The change removes the ethers dependency for signing, simplifies the code by using u128 instead of U256 for values, and moves address validation to the tinywallet crate. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The EVM fields for status, block number, gas used, and effective gas price are all within the range of a u128, so using hex_to_u128 instead of hex_to_u256 avoids unnecessary overhead and potential conversion issues. This change simplifies the code by removing the need for the as_u64() call on block numbers and the is_zero() check on status values. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `as_u64()` method silently truncates values larger than `u64::MAX`, which could cause incorrect block number calculations. The change replaces it with `u64::try_from()` and falls back to `u64::MAX` on overflow, ensuring that out-of-range block numbers are handled safely rather than producing silently wrong confirmation counts. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ction The Tron wallet module now delegates transaction signing to the shared `sign_transaction` function instead of performing ECDSA recoverable signing inline. This change also replaces the `bitcoin::secp256k1` dependency with `k256` for key operations and adds a `compressed_public_key` helper, ensuring the module verifies the transaction content against the expected recipient and txID before signing, rather than blindly signing whatever the node returns. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The transaction verification was comparing the wrong address for TRC20 transfers, because the `to_address` from the quote always points to the token contract rather than the actual recipient. A new `verified_recipient` variable now holds the contract address for TRC20 transfers and the direct recipient for native transfers, so the signature check matches the address that the transaction actually pays. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace the manual Bitcoin transaction construction and signing logic in `execute_btc_quote` with a call to the `tinywallet` module, which now handles both the transaction specification and signing. The `derive_btc_private_key` function returns raw key bytes instead of Bitcoin-specific key types, and the UTXO selection remains in this crate to keep fee policy and source logic local while ensuring the module's reselection matches the same algorithm. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…odule The change removes the `bitcoin` crate imports and the `script_pubkey_for_addr` helper function that were no longer used after the wallet was migrated to the `tinywallet` crate. The test for key derivation is updated to use `tinywallet` directly for verification instead of the removed `bitcoin` types, keeping the same known-good address vector. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed the `std::str::FromStr` import from the BTC and EVM chain modules and the `sha2::{Digest, Sha256}` import from the Tron chain module, as these imports were no longer used in the respective files.
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…allet Replace the `derive_evm_signer` function to return raw secret bytes and a string address instead of an ethers wallet, and remove the unused EIP-712 and EIP-3009 helper functions. This change eliminates the dependency on the ethers signer crate for x402 operations, relying instead on the same `tinywallet::key` derivation used elsewhere in the wallet domain, ensuring consistency and reducing the dependency footprint. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…yment Switched the EVM payment construction from using the `ethers` wallet and address types to raw `k256` signing and byte-array address handling. This removes the dependency on `ethers` for signing, making the module lighter and more portable, while keeping the same EIP-3009 authorization flow. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ion tests The inline `cargo test` commands in comments for ignored wallet integration tests were missing the `--ignored` flag, which is required to execute tests annotated with `#[ignore]`. This change adds the flag to all affected test commands across BTC, EVM, Tron, and execution test files, ensuring developers can correctly run these tests in isolation. Auto-committed-on: dragonfly
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/web3/wallet/chains/tron.rs`:
- Around line 315-320: Add debug or trace diagnostics around the validation flow
containing compressed_public_key and tron_transaction_spec: log both successful
validation and rejection outcomes with quote_id, transaction ID, and transfer
kind. Use grep-friendly structured fields, and explicitly exclude raw_data_hex,
mnemonic, key bytes, and other secrets or full PII from all messages.
- Around line 114-136: Replace the byte-substring checks in the Tron transaction
verification flow with parsing of the canonical contract payload from
raw_data_hex. Update TransactionSpec::Tron and its callers as needed to carry
the expected native amount or TRC20 parameter, then compare the parsed
recipient, exact amount, or decoded TRC20 ABI argument before sign_transaction
is reached; do not permit a transaction to pass based only on bytes occurring
elsewhere in raw.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3d3e236-053c-461f-9e57-f5c8d9a4463a
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
src/openhuman/modules/mod.rssrc/openhuman/modules/wallet.rssrc/openhuman/modules/wallet_tests.rssrc/openhuman/web3/wallet/chains/btc.rssrc/openhuman/web3/wallet/chains/evm.rssrc/openhuman/web3/wallet/chains/tron.rssrc/openhuman/web3/wallet/execution.rssrc/openhuman/web3/wallet/execution_tests.rssrc/openhuman/web3/x402/ops.rssrc/openhuman/web3/x402/x402_tests.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- src/openhuman/modules/wallet_tests.rs
- src/openhuman/web3/wallet/execution_tests.rs
- src/openhuman/modules/wallet.rs
- src/openhuman/web3/x402/ops.rs
- src/openhuman/web3/wallet/chains/btc.rs
- src/openhuman/web3/x402/x402_tests.rs
…er verification The Tron transaction verification previously used a naive substring search on the raw transaction bytes to find the recipient address and transfer parameters, which could produce false positives. This change introduces proper protobuf field parsing to extract and validate the specific contract fields, ensuring that native transfers and TRC20 transfers are verified against the correct protobuf fields rather than relying on byte-level pattern matching. Auto-committed-on: dragonfly
Add low-level protobuf parsing functions to extract contract type, type URL, and payload from a Tron transaction's raw data. This is needed to support decoding Tron contract parameters for signing and verification workflows. Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
Add debug logging for transaction validation outcomes in `execute_tron_quote` to distinguish accepted and rejected transfers by kind, quote ID, and transaction ID. Refine error messages in `tron_transaction_spec` tests to be more specific about mismatched amounts and transfer data, and add a test case ensuring that a decoy value in an unrelated raw-data field does not bypass validation when the selected contract specifies a different payment. Auto-committed-on: dragonfly
Renamed `native_raw` to `native_raw_hex` in the test helper to make it explicit that the variable holds a hex-encoded string rather than raw bytes, improving code readability. Auto-committed-on: dragonfly
The `one_bytes` function had an unnecessary explicit lifetime on the `fields` parameter that was already covered by the function's generic lifetime `'a`. Removing it eliminates a compiler warning and simplifies the signature without changing any behaviour. Auto-committed-on: dragonfly
The function signature for `one_bytes` was unnecessarily split across multiple lines, making it harder to read. This change consolidates the parameters onto a single line to improve code clarity without altering any behavior. Auto-committed-on: dragonfly
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/web3/wallet/chains/tron.rs`:
- Around line 128-163: Update tron_transaction_spec to validate node-controlled
transaction values: in the TRC20 TriggerSmartContract path, parse and reject any
non-zero call_value, and validate Transaction.raw.fee_limit as either absent or
equal to TRC20_FEE_LIMIT_SUN. Preserve the existing recipient and transfer-data
checks while returning an appropriate validation error for either mismatch.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6841fb31-c967-48be-afca-146e44fe6a55
📒 Files selected for processing (1)
src/openhuman/web3/wallet/chains/tron.rs
Added checks for the TRC20 call_value and fee_limit fields when verifying a TRC20 transfer, rejecting transactions that include a non-zero call_value or a fee_limit that differs from the expected constant. Also refactored the protobuf field parsing to expose an optional_varint helper, which the new validation uses, and adjusted parse_single_tron_contract to accept pre-parsed raw fields instead of re-parsing them internally. Auto-committed-on: dragonfly
…tions Extend the test helper to accept optional call_value and fee_limit parameters, and add test cases that verify the transaction spec correctly rejects TRC20 transfers with a non-zero call_value or an unexpected fee_limit. This ensures the validation logic catches these mismatches before they reach the network. Auto-committed-on: dragonfly
Reformat several function calls that were split across multiple lines to fit within the line length limit, improving code readability without changing any behavior. Auto-committed-on: dragonfly
The status_of function now checks whether modules are disabled before consulting the resolution cache, so a module whose resolution is cached as Ready or Failed correctly reports Unsupported when the feature is turned off. The corresponding test is updated to accept Ready as a valid state, since the process-global cache may already contain a loaded module from a previous test. Auto-committed-on: dragonfly
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/openhuman/modules/ops_tests.rs (1)
42-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for disabled-module precedence.
offline_config()setsconfig.modules.enabled = trueat Line 16, so this test never reaches the new disabled branch insrc/openhuman/modules/ops.rsLines 314-317. Add a separate case with modules disabled. AssertModuleState::Unsupportedand the detail"modules are disabled in configuration", including when the resolution cache containsReadyorFailed.Example regression test
+#[test] +fn disabled_modules_override_cached_resolution() { + let mut config = offline_config(); + config.modules.enabled = false; + + for status in list(&config) { + assert_eq!(status.state, ModuleState::Unsupported); + assert_eq!( + status.detail.as_deref(), + Some("modules are disabled in configuration") + ); + } +}This review uses the changed disabled-state branch in
src/openhuman/modules/ops.rs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/modules/ops_tests.rs` around lines 42 - 46, Add a separate regression test in the ops tests that configures modules as disabled, then asserts the reported state is ModuleState::Unsupported with detail "modules are disabled in configuration". Cover both preexisting resolution-cache states, Ready and Failed, ensuring disabled configuration takes precedence regardless of cached resolution.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/modules/ops_tests.rs`:
- Around line 47-51: Update the status assertion in the relevant ops test to
also accept ModuleState::Failed, and require status.detail.is_some() whenever
the state is Failed or Unsupported. Preserve acceptance of Available and Ready
while accounting for cached resolution failures.
---
Nitpick comments:
In `@src/openhuman/modules/ops_tests.rs`:
- Around line 42-46: Add a separate regression test in the ops tests that
configures modules as disabled, then asserts the reported state is
ModuleState::Unsupported with detail "modules are disabled in configuration".
Cover both preexisting resolution-cache states, Ready and Failed, ensuring
disabled configuration takes precedence regardless of cached resolution.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 392325e7-4aca-4eec-a364-84c67645df06
📒 Files selected for processing (2)
src/openhuman/modules/ops.rssrc/openhuman/modules/ops_tests.rs
Auto-committed-on: dragonfly
The CI workflow was setting CARGO_LLVM_COV_TARGET_DIR to place coverage profiles in a temporary directory, but this also redirected all build artifacts. The change now uses LLVM_PROFILE_FILE_NAME instead, which only overrides the generated profile filename while keeping build output in the default target directory. Auto-committed-on: dragonfly
Replace the indirect `cargo llvm-cov` invocation with a direct `cargo test` call, sourcing the coverage environment from `cargo llvm-cov show-env` instead. This prevents the tool from overwriting the custom `LLVM_PROFILE_FILE` path and gives finer control over where raw profiles are written, making the profile collection step more reliable. Auto-committed-on: dragonfly
…alization Replace the hardcoded Tron transaction stubs in the mock server with proper protobuf-encoded responses that reflect the actual request payload. This ensures the wallet execution surface tests validate real serialization logic rather than passing through opaque dummy data, making the round-trip tests meaningful for Tron and TRC20 flows. Also mark the wallet-dependent tests as ignored by default since they require an installed tinywallet artifact. Auto-committed-on: dragonfly
The mock_tron_varint_field call in the Tron trigger test was unnecessarily split across four lines, making the test harder to read. The call now fits on a single line, improving readability without changing any behaviour. Auto-committed-on: dragonfly
The profile directory was changed from /tmp to /dev/shm to avoid dropped LLVM profile writes on hosted container jobs, where both the workspace bind mount and the container overlay have been observed to silently fail profile writes. Using the container-local in-memory filesystem ensures reliable profile storage for the small raw counter files. Auto-committed-on: dragonfly
The sccache wrapper was being inherited by the llvm-cov test process, which could return cached objects compiled without coverage instrumentation. This caused test binaries to pass without writing raw profile data. The change unsets RUSTC_WRAPPER to ensure all compiled objects are instrumented for coverage. Auto-committed-on: dragonfly
Moves Bitcoin, EVM, Solana and Tron transaction building out of this binary and into the loadable
tinywalletmodule, the waydocumentsmoved intotinydocs. The signing key never leaves this process.Depends on tinywallet #10 / #11, released as v0.2.0. The gitlink is pinned to that tag and the eleven digests in
registry.rsare copied verbatim from itschecksum.toml.Measured
bitcoin,ethers-core,ethers-signers,secp256k1and the nativesecp256k1-sysC build are all gone — proven withscripts/assert-shed.sh, notcargo tree -i. Kernel floor unchanged (282 names / 2 native).The structural reason this took a tinywallet change first:
bitcoinhad two parents, this crate and tinywallet, whosekeyfeature requiredbtc. Cutting our own edge shed nothing. tinywallet now reachesbitcoinonly throughtx, which lives exclusively in the module.Keys stay here
k256is a new direct dependency and costs nothing — it was already in the graph beneathcoins-bip32, which derives the key being signed with.AttachSignaturere-sends the fields rather than a handle, so the module keeps no state between calls: no store, no bounds, no expiry.A loaded module shares this address space, so this is not hard isolation and is not claimed as such — a hostile module could read the seed from process memory anyway. It is a refusal to widen a boundary that already exists, for the price of one extra in-process round trip.
Two behaviour changes worth reviewing
Tron now verifies what the node returns before signing it. The module recomputes
txID == sha256(raw_data)and checks the recipient appears in the bytes. The previous code signed whatevercreatetransactionhanded back. This caught the test fixtures immediately — they had arbitrarytxIDs and no recipient, and had been signed without complaint for as long as they existed. They are now well-formed node responses.A TRC20 transaction is verified against the contract, not the recipient. A token transfer pays the contract and carries the real recipient padded inside the calldata, without the
41prefix that appears for a native transfer — so verifying againstquote.to_addresswould break every token transfer. Handled per transfer kind.UTXO selection was safe to delegate: I checked rather than assumed, and
select_utxoshere andselect_coinsin tinywallet are the same largest-first algorithm down to the 546-sat dust rule.Test coverage, honestly
8 mock-server tests are
#[ignore]d. Not because they fail — each passes in isolation, exercising the full path: download, digest verification,dlopen, and a real signed transaction. They cannot run together, because tinybus never unloads a module and the module bus belongs to whichever tokio runtime created it; the second#[tokio::test]finds a dead broker. Same constraint tinydocs documents. Each ignore attribute carries the command that runs it.Everything else is green: 140 web3 tests, 51 module tests, clippy clean on the product profile, Feature Forwarding Gate and kernel-floor ratchet both pass.
Not claimed
This does not make builds faster. The
documentsport shed 60 crates for zero wall-clock change — measured cold with sccache disabled, the build is critical-path-bound at ~200% CPU on 14 cores and the shed crates are parallel leaves. The win here is supply-chain surface, binary size, and one fewer native C toolchain build.Summary by CodeRabbit