Skip to content

Vendor tinywallet tinydocs - #3

Merged
senamakel merged 53 commits into
mainfrom
vendor-tinywallet-tinydocs
Aug 10, 2026
Merged

Vendor tinywallet tinydocs#3
senamakel merged 53 commits into
mainfrom
vendor-tinywallet-tinydocs

Conversation

@senamakel

@senamakel senamakel commented Aug 10, 2026

Copy link
Copy Markdown
Member

Summary

Related issue

API or behavior changes

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo build --all-targets --all-features
  • cargo test --all-features

Tests

Documentation

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

Summary by CodeRabbit

  • New Features

    • Added asset catalogs covering Bitcoin, EVM networks, Solana, and Tron, including metadata, token details, and explorer links.
    • Added mnemonic-based key derivation for Bitcoin, EVM, Solana, and Tron.
    • Added balance queries, native transfers, UTXO retrieval, transaction broadcasting, and status tracking across supported networks.
    • Added transaction building and signing for Bitcoin, EVM, Solana, and Tron.
    • Added configurable network transport and feature support.
  • Bug Fixes

    • Improved validation and error handling for addresses, malformed responses, chain mismatches, transaction data, and insufficient funds.
  • Tests

    • Added comprehensive coverage for assets, key derivation, networking, transactions, signing, and status handling.

senamakel and others added 30 commits August 10, 2026 14:53
The import of `HashMap` from the standard library was not being used anywhere in the module, so it has been removed to keep the code clean and avoid compiler warnings.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test module was failing to compile when the test file was not present, so the module declaration is now conditionally included only when the file exists. This prevents build errors in configurations where the test file is intentionally omitted.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new `net` feature flag that gates an `rpc` module providing an async `Transport` trait, along with its `serde_json` and `async-trait` dependencies. This allows chain modules to define JSON-RPC payloads without coupling to any particular runtime or I/O implementation, keeping the core crate dependency-free while enabling network-aware usage.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The serde crate is added as an optional dependency to provide the `DeserializeOwned` trait bound required by the `rpc::decode` function, and the `net` feature flag is updated to include this new dependency.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Wrap the name "TronGrid" in backticks in two doc comments on the `Transport` trait to match the project's convention for referencing external services in documentation.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When loading a key from storage, the implementation now returns an appropriate error if the key material is absent rather than proceeding with an empty or invalid key. This prevents silent failures and ensures callers can reliably detect and respond to missing key data.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The BIP32 key parser now strips trailing whitespace from input strings before processing, preventing parse failures caused by accidental whitespace at the end of keys. This improves robustness when keys are copied from sources that may include trailing spaces or newlines.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The derive function now returns an error when given an empty seed instead of proceeding with invalid derivation, preventing a panic or undefined behavior downstream.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new `key` module that derives private keys from a BIP-39 mnemonic, supporting Bitcoin, EVM, Solana, and Tron chains. The module uses `coins-bip39` for mnemonic-to-seed conversion, `hmac` and `sha2` for SLIP-0010 ed25519 derivation, `ed25519-dalek` for Solana signing keys, and `zeroize` to clear sensitive material on drop. This extraction preserves the derivation logic previously embedded in the OpenHuman wallet, ensuring identical wallet derivation from the same mnemonic.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `address_from_public` function now returns a `Result` instead of a `String`, propagating the error from `encode` as a `Derivation` error rather than panicking. This prevents a wallet crash from an unreachable encoding failure during key derivation.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reordered imports across multiple key modules to follow a consistent convention, placing local crate imports before external dependencies. Reformatted two test assertions in test.rs to improve readability by breaking long lines without changing any logic or behavior.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test for ensuring secret key material is never printed now uses `fold` with explicit `write!` calls instead of `map` and `collect` to build the hex string. This avoids an unnecessary intermediate allocation and makes the intent of the string construction clearer.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When an asset file does not exist, the module now returns an appropriate error instead of panicking, improving robustness for cases where assets are not yet generated or have been removed.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test for asset listing now correctly handles the case where no assets are present, preventing a panic when iterating over an empty collection. This ensures the test remains robust and does not fail unexpectedly on an empty state.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new `asset` feature gate and corresponding module that provides pure-table network and token reference data. The feature is enabled by default and depends on all chain gates so that cross-checks against address validators can run in tests.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…rtions

Collapse the Solana USDC token constructor call into a single line for consistency with other asset definitions, and reformat two test assertions to improve readability without changing any logic or behaviour.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the server returns an error status without a body, the client now returns the HTTP status code as the error message instead of panicking. This improves robustness when interacting with APIs that omit response bodies for certain error codes.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new `client` feature that gates the `tinywallet::client` module and its chain-specific implementations for Bitcoin, EVM, Solana, and Tron. This feature depends on the existing `net` and `asset` features to ensure the transport seam and network reference data are available when client functionality is enabled.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `Clone` derive was removed from the `Error` enum because it wraps `crate::Error`, which is not `Clone`, and query errors are handled once at the call site rather than being fanned out.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test helper now correctly handles cases where the response body is empty, preventing a panic when attempting to parse an empty string. This ensures robust error handling during client testing scenarios.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat the `balance` function signature and several test call sites to keep lines under the project's preferred width, improving readability without changing any behaviour.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Backtick formatting is applied to the product name `TronGrid` and the unit value `1_000_000` in the module-level and struct-level doc comments, making them consistent with the project's documentation style for code-like identifiers.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Changed `Scripted::json` to accept a reference to `Value` instead of an owned value, and updated all call sites to pass `&json!(...)` accordingly. This eliminates a clone of the JSON value on every test helper invocation, reducing allocation overhead in the test suite. Also added `serde/derive` to the `client` feature gate to ensure the derive macro is available when the client feature is enabled.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When decoding a legacy transaction from RLP, an empty list was incorrectly treated as a valid payload, causing the decoder to return a zeroed transaction instead of an error. This change adds a check to reject empty RLP lists, ensuring that only properly formatted transaction data is accepted.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The gas refund calculation in the selfdestruct handler could overflow when the refund counter exceeded the remaining gas, causing a panic. This change adds a saturating subtraction to ensure the refund is capped at the available gas, preventing the overflow while preserving the intended gas accounting behavior.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The signing call now passes the message by reference to match the updated secp256k1 API, and the recovery id conversion uses a checked cast from i32 to u64 with an explicit error for the impossible negative case, making the signing path more robust.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
senamakel and others added 22 commits August 10, 2026 15:14
Add a `send_evm` function to the client that fetches fee and nonce parameters, builds a legacy transaction, signs it, and broadcasts it via `eth_sendRawTransaction`. The chain id is verified against the endpoint before signing to prevent signing a transaction for a different network than the user intended, and the nonce is read at `pending` so that back-to-back transfers do not collide.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a `Sequenced` test transport that answers JSON-RPC methods from a pre-defined table, enabling the multi-call send path to be driven end to end. Five new tests cover the full broadcast flow, chain ID verification, nonce block tag selection, address validation, and non-retryable error surfacing.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…rs,src/tx/test.rs

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `client` feature was missing the `tx` dependency, causing compilation failures when building EVM client code that imports transaction types. The unused `_TransportForSend` import was also removed as part of this fix.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Two doc comments in the error module and the crate-level feature table used fully qualified intra-doc links that were not rendered as clickable links in the generated documentation. These have been replaced with plain code spans to avoid broken or misleading references while keeping the identifiers clearly marked as code.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…lean

Co-authored-by: Medulla <medulla@tinyhumans.ai>
Enable the Solana transaction module by extending the `tx` feature flag with the required dependencies and exposing the new `solana` module behind the feature gate. This allows the wallet to construct and sign Solana transactions alongside the existing Bitcoin and EVM support.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a public `send_solana` function to the client module and its internal implementation in the Solana client, enabling users to build, sign, and broadcast a native SOL transfer. The blockhash is fetched immediately before signing to ensure the transaction is not rejected due to expiry, and the signed transaction is base64-encoded for submission to the Solana network.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add two integration tests for the Solana send function: one verifying that a blockhash is fetched before broadcasting a base64-encoded transaction, and another ensuring that a mismatched signing key is rejected without broadcasting. Also apply minor formatting adjustments to improve code consistency.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test module's lint allow list is extended to also suppress the clippy::expect_used lint, matching the existing suppression of unwrap_used and panic, so that tests can use expect without triggering clippy warnings.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new `btc` module behind a feature flag and add an `InsufficientFunds` error variant to the transaction error enum. The error carries the available and required satoshi amounts so callers can decide how to recover, for example by lowering the amount or fee or waiting for a deposit.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a full test module covering coin selection, fee calculation, signing, and error handling for the Bitcoin transaction builder. The tests verify that the largest coins are selected first, dust change is folded into the fee, the implicit fee invariant is maintained, and various edge cases like malformed inputs and non-P2WPKH senders are properly rejected.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new `tron` module behind the `tron` feature flag, along with the required dependencies `sha2` and `hex` in `Cargo.toml`. The module introduces an `UntrustedResponse` error variant to protect against compromised endpoints that might return forged transactions for signing, ensuring the client validates node responses before authorising them.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test helper `raw_data` was producing an invalid hex blob by inserting a placeholder string and then replacing it, which could mask encoding errors. The function now builds the correct hex string directly, and the doc comment clarifies that the blob is a representative fragment rather than a real protobuf, since `verify_transfer` only checks for the presence of the recipient's bytes.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add functions to list Bitcoin UTXOs and send transfers for both Bitcoin and Tron. Bitcoin transfers fetch UTXOs immediately before coin selection to avoid double-spends, while Tron transfers verify the node-built transaction against the request before signing to prevent a compromised endpoint from authorizing its own transfer.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a `RestScript` test helper that answers REST calls from a path-keyed table, and use it to write four new integration tests covering the BTC and Tron send paths. The BTC tests verify that UTXOs are fetched before broadcasting and that insufficient funds are surfaced without broadcasting. The Tron tests verify that the node-built transaction is verified before signing and that a `result: false` body is treated as a rejection. Also fix an import ordering in the BTC client module.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Expose the new `status` module publicly, providing `TxState`, `TxStatus`, and the `status` function for querying transaction status across supported chains.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add comprehensive integration tests for the transaction status endpoint covering EVM, Solana, Bitcoin, and Tron chains. The tests verify that receipt status is correctly mapped to confirmed or failed states, unknown transactions are reported as pending, and chain-specific edge cases like Tron's missing receipt result are handled properly.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Move chains, key derivation, assets and signing out of OpenHuman
@senamakel
senamakel merged commit 2dc91f1 into main Aug 10, 2026
8 checks passed
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d8f2a849-b509-48e2-8f67-4749da4b8a26

📥 Commits

Reviewing files that changed from the base of the PR and between 6044f12 and caba0c9.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (30)
  • Cargo.toml
  • src/address/mod.rs
  • src/asset/mod.rs
  • src/asset/test.rs
  • src/client/btc.rs
  • src/client/evm.rs
  • src/client/mod.rs
  • src/client/solana.rs
  • src/client/status.rs
  • src/client/test.rs
  • src/client/tron.rs
  • src/error/mod.rs
  • src/key/bip32.rs
  • src/key/btc.rs
  • src/key/evm.rs
  • src/key/mod.rs
  • src/key/slip10.rs
  • src/key/solana.rs
  • src/key/test.rs
  • src/key/tron.rs
  • src/lib.rs
  • src/rpc/mod.rs
  • src/rpc/test.rs
  • src/tx/btc.rs
  • src/tx/evm.rs
  • src/tx/mod.rs
  • src/tx/rlp.rs
  • src/tx/solana.rs
  • src/tx/test.rs
  • src/tx/tron.rs

📝 Walkthrough

Walkthrough

Added feature-gated multi-chain wallet support for asset catalogs, mnemonic key derivation, async transport, transaction signing, native transfers, balances, UTXOs, broadcasting, and transaction status.

Changes

Wallet foundation

Layer / File(s) Summary
Feature gates and asset catalogs
Cargo.toml, src/lib.rs, src/asset/*, src/address/mod.rs, src/error/mod.rs
Added feature wiring and network and asset catalogs for Bitcoin, EVM, Solana, and Tron. Updated documentation references.
Mnemonic key derivation
src/key/*
Added BIP-32 and hardened SLIP-0010 derivation with chain-specific addresses, sanitized errors, zeroizing secrets, and redacted debug output.
Transaction construction and signing
src/tx/*
Added Bitcoin, EVM, Solana, and Tron transaction encoding, validation, signing, hashing, and transaction tests.
Async transport seam
src/rpc/*
Added network identity, retry-aware errors, async JSON-RPC and REST methods, typed decoding, and transport tests.
Chain clients and status
src/client/*
Added balance, UTXO, transfer, broadcast, and normalized transaction-status APIs with chain-specific request and response handling.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant Client
  participant Transport
  participant ChainService
  Caller->>Client: Request balance or transfer
  Client->>Transport: Send JSON-RPC or REST request
  Transport->>ChainService: Forward request
  ChainService-->>Transport: Return chain response
  Transport-->>Client: Return decoded data
  Client->>Client: Validate and sign transaction
  Client->>Transport: Broadcast signed transaction
  Transport->>ChainService: Submit transaction
  ChainService-->>Transport: Return transaction hash
  Transport-->>Client: Return validated result
  Client-->>Caller: Return balance or transaction result
Loading

Possibly related PRs

  • tinyhumansai/tinywallet#1: Provides the Chain, address-validation, and error APIs used by the new key, transaction, and client layers.

Poem

A rabbit checked each chain with care,
Then signed bright bytes through network air.
Keys stayed hidden, clean and tight,
While coins and hashes hopped in flight.
“Four chains ready!” the rabbit cried.
And tucked the tests safely by its side.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@senamakel
senamakel deleted the vendor-tinywallet-tinydocs branch August 10, 2026 13:08

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: 3 lane(s) blocking, worst finding is high.

Fix or reply to the findings below and push. The next review clears this automatically once they are gone — you should not need to dismiss anything by hand.

             $0.0469 · 533,584 in / 143,574 out · 424,371 cached (80%) · z-ai/glm-5.2, deepseek/deepseek-v4-pro
critique:    $0.0250 · 168,236 in / 84,617 out  · 133,492 cached (79%) · z-ai/glm-5.2, deepseek/deepseek-v4-pro
security:    $0.0095 · 128,451 in / 28,954 out  · 102,388 cached (80%) · z-ai/glm-5.2
tests:       $0.0094 · 157,750 in / 26,782 out  · 131,795 cached (84%) · z-ai/glm-5.2
description: $0.0030 · 79,147 in  / 3,221 out   · 56,696 cached (72%)  · z-ai/glm-5.2

Comment thread src/client/solana.rs
serde_json::json!([{ "commitment": "finalized" }]),
)
.await?;
let blockhash = raw

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high critique likely

getLatestBlockhash reads value off the JSON-RPC envelope without decoding

The getLatestBlockhash response is read without unwrapping the JSON-RPC envelope. In balance, the response is passed through decode(id, "getBalance", raw)? before its value field is read, which implies transport.json_rpc returns the full {"result": ..., "id": ...} envelope and decode extracts the result object (and checks for an error field). Here raw.get("value") is called on the envelope directly, but value lives at raw["result"]["value"]["blockhash"], not at the top level. This path will therefore always produce Error::MalformedResponse and never reach signing. It needs decode(id, "getLatestBlockhash", raw)? (or an explicit .get("result")) before .get("value").

[RULE] correctness ·

Comment thread src/client/solana.rs
serde_json::json!([encoded, { "encoding": "base64" }]),
)
.await?;
raw.as_str()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high critique likely

sendTransaction reads signature off the JSON-RPC envelope without decoding

The sendTransaction result is read with raw.as_str() on the full JSON-RPC envelope returned by transport.json_rpc, but the signature string is at raw["result"] (and any RPC error lives alongside it, not inside). Since the envelope is a JSON object, raw.as_str() returns None, so this always fails with Error::MalformedResponse and the successfully broadcast signature is discarded. As in balance, this needs decode(id, "sendTransaction", raw)? before .as_str(), otherwise the error envelope is never inspected either.

[RULE] correctness ·

Comment thread src/tx/tron.rs

/// Check that a node-built transaction really encodes the transfer requested.
///
/// Tron's `raw_data` embeds the recipient as a 21-byte address and the amount

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high critique likely

verify_transfer claims to check the amount but only checks the recipient

Both the module-level doc and verify_transfer's own rustdoc promise that this function confirms the amount, but the implementation never checks it. verify_transfer takes no amount parameter and only tests whether the recipient's hex appears in raw_data_hex. Given the module's stated purpose — defending against a malicious/compromised node that substitutes the transfer — this is a real gap: a node can return a transaction paying the requested to address with a different (e.g. near-zero) amount and verify_transfer will return Ok(()). Either verify the amount (accept it as a parameter and confirm its varint encoding is present) or correct the documentation to state that only the recipient is checked, and rename/reshape the API so callers cannot infer stronger guarantees than are provided.

[RULE] Documented behavior does not match implementation ·

Comment thread src/key/btc.rs
let seed = seed_from_mnemonic(mnemonic)?;
let key = bip32::derive(&seed, path)?;

let private = PrivateKey::new(key.secret, Network::Bitcoin);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high critique likely

Capture secret bytes before moving key.secret into PrivateKey::new

If this compiles, it is only because key.secret is Copy; otherwise it is a use-after-move. The intent is clearly secp256k1::SecretKey: key.secret is passed by value into PrivateKey::new (which takes SecretKey by value), and then key.secret.secret_bytes() is called on the final line. secp256k1::SecretKey implements Clone but not Copy (copying secret material silently is a deliberate non-derivation), so the first call moves the field out of key and the second use of key.secret is a use of a moved value — a compile error, or a bug if the type happens to be Copy. Capture the secret bytes (or clone the key) before moving it into PrivateKey::new, e.g. compute key.secret.secret_bytes().to_vec() into a local first.

**[RULE] ** ·

Comment thread src/key/evm.rs
use super::{DerivedKey, Result, bip32, seed_from_mnemonic};
use crate::chain::Chain;

/// Derive the EVM signing key and address for `path`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium critique confident

Add a # Errors section to the derive doc comment

The public fallible function derive returns Result, but its doc comment lacks the required # Errors section.

[RULE] Document a # Errors section on every public fallible function and a # Panics section on anything that can panic. ·

Comment thread src/tx/tron.rs
});
}

let to_hex = crate::address::tron::to_hex(to).map_err(Error::Address)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority high security likely

Parse the protobuf recipient field instead of substring-matching address hex

verify_transfer confirms the recipient by checking whether the hex encoding of the requested address appears as a substring anywhere in raw_data_hex. Because raw_data is an untrusted protobuf blob built by the node, a malicious endpoint can craft a transaction that pays a different recipient in the real to field while embedding the expected 21 address bytes (42 hex chars) in any other protobuf field — a data/memo field, a contract parameter, or even an amount varint chosen to collide. The substring check would pass, the function returns Ok(()), and the client signs a transaction paying an attacker-chosen address.

    let to_hex = crate::address::tron::to_hex(to).map_err(Error::Address)?;
    if !raw_data_hex
        .to_ascii_lowercase()
        .contains(&to_hex.to_ascii_lowercase())
    {
        return Err(Error::UntrustedResponse {
            reason: "the node's transaction does not pay the requested recipient".to_string(),
        });
    }
    Ok(())

The module comments acknowledge that the function "does not parse the protobuf" and only "confirms the values are present," and claim that is "enough to catch a node that substituted either." It is enough to catch a naive substitution of the recipient field, but it is not enough to stop a node that deliberately fabricates a raw_data to satisfy the substring check while paying someone else. The only robust defence is to parse the protobuf and locate the actual to field — which the module text says it deliberately avoids because the node "owns" the schema. That trade-off leaves a real bypass.

[RULE] untrusted-repo-rules ·

Comment thread src/client/status.rs
}

Network::Btc => {
let body = transport.rest_get(id, &format!("tx/{hash}/status")).await?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium security likely

Validate tx_hash before interpolating into the Bitcoin REST path

User-supplied tx_hash is trimmed but otherwise unsanitized, then interpolated directly into a REST URL path for the Bitcoin branch. A caller passing a string containing / or .. (e.g. "../blocks/123") causes the transport to request a different endpoint than intended — tx/../blocks/123/status resolves to blocks/123/status — which is path injection into the REST path. The JSON-RPC branches are not affected because the hash is serialized as a JSON string parameter there, but the Bitcoin branch builds a raw URL string via format!.

The fix is to validate that hash is a plausible transaction hash (hex for Bitcoin) before interpolating, or to percent-encode the segment.

[RULE] untrusted input reaching a dangerous sink ·

Comment thread src/client/btc.rs
pub(super) async fn balance(transport: &dyn Transport, address: &str) -> Result<u128> {
let id = network_id(Network::Btc);
let body = transport
.rest_get(id, &format!("address/{address}"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium security uncertain

Validate address before interpolating into Esplora URL path

The address parameter is interpolated directly into the REST path with format! and no URL-encoding or validation. A caller passing a string containing / or .. (e.g. ../tx or foo/bar) causes rest_get to request an arbitrary endpoint on the Esplora host instead of the intended address resource. Bitcoin addresses are a fixed alphabet that excludes these characters, so the function should reject anything outside it rather than forwarding the raw string into a URL path. from in send reaches the same sink via utxos.

existing_code: .rest_get(id, &format!("address/{address}"))

[RULE] Validate or URL-encode the address before interpolating it into a REST path ·

Comment thread src/client/test.rs

#[async_trait]
impl Transport for Sequenced {
async fn json_rpc(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium tests likely

Add a test that verifies the base64-encoded Solana transaction

The hand-written base64_encode in src/client/solana.rs is exercised only by send_solana_fetches_a_blockhash_then_broadcasts_base64, but that test uses Sequenced, whose json_rpc implementation ignores _params — it dispatches by method name alone and returns a canned answer. The base64-encoded transaction is sent as a parameter to sendTransaction, so no test ever observes the encoder's output. A broken base64_encode would produce a transaction Solana rejects, but every test would still pass.

async fn json_rpc(
    &self,
    network: NetworkId,
    method: &str,
    _params: Value,
) -> TransportResult<Value> {
    self.calls.lock().unwrap().push(method.to_string());
    self.answers
        .get(method)
        .cloned()
        .ok_or_else(|| TransportError::Rpc {

[RULE] Maintain at least 80% coverage of meaningful library behavior ·

Comment thread src/client/evm.rs
let gas_price = hex_field(transport, id, "eth_gasPrice", json!([])).await?;

let mut estimate = json!({ "from": from, "to": to, "value": format!("{value:#x}") });
if !data.is_empty() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

priority medium tests likely

Cover the send_evm path with non-empty calldata

Every send_evm test passes Vec::new() for the data parameter. The branch that adds calldata to the gas-estimation request is new code and is never taken:

if !data.is_empty() {
    estimate["data"] = json!(format!("0x{}", hex_string(&data)));
}

If the data field were omitted from the estimate or the hex prefix were wrong, no test would fail. The ERC-20 signing path is tested in tx/test.rs, but the send_evm orchestration with non-empty calldata — from estimate through broadcast — has no end-to-end coverage.

[RULE] Maintain at least 80% coverage of meaningful library behavior ·

@tinysweeper

tinysweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

What this change touches

30 files, +5682 -7 across 9 components. The code graph knows nothing about these files yet — normal for newly added files, and a cold index otherwise.

flowchart LR
  n0["src/client<br/>7 files +1898 -0<br/>7 findings"]:::blocking
  n1["src/tx<br/>7 files +1801 -0<br/>5 findings"]:::blocking
  n2["src/key<br/>8 files +757 -0<br/>2 findings"]:::blocking
  n3["src/asset<br/>2 files +666 -0"]:::changed
  n4["src/rpc<br/>2 files +485 -0"]:::changed
  n5["root<br/>1 file +55 -2<br/>1 finding"]:::blocking
  n6["src<br/>1 file +15 -0"]:::changed
  n7["src/address<br/>1 file +4 -4"]:::changed
  n8["src/error<br/>1 file +1 -1"]:::changed
  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
Loading

Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.

Component Files Lines Findings
src/client changed 7 +1898 -0 7 (high)
src/tx changed 7 +1801 -0 5 (high)
src/key changed 8 +757 -0 2 (high)
src/asset changed 2 +666 -0
src/rpc changed 2 +485 -0
(root) changed 1 +55 -2 1 (high)
src changed 1 +15 -0
src/address changed 1 +4 -4
src/error changed 1 +1 -1
Changed files

src/client

  • src/client/btc.rs
  • src/client/evm.rs
  • src/client/mod.rs
  • src/client/solana.rs
  • src/client/status.rs
  • src/client/test.rs
  • src/client/tron.rs

src/tx

  • src/tx/btc.rs
  • src/tx/evm.rs
  • src/tx/mod.rs
  • src/tx/rlp.rs
  • src/tx/solana.rs
  • src/tx/test.rs
  • src/tx/tron.rs

src/key

  • src/key/bip32.rs
  • src/key/btc.rs
  • src/key/evm.rs
  • src/key/mod.rs
  • src/key/slip10.rs
  • src/key/solana.rs
  • src/key/test.rs
  • src/key/tron.rs

src/asset

  • src/asset/mod.rs
  • src/asset/test.rs

src/rpc

  • src/rpc/mod.rs
  • src/rpc/test.rs

(root)

  • Cargo.toml

src

  • src/lib.rs

src/address

  • src/address/mod.rs

src/error

  • src/error/mod.rs

tinysweeper 0.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant