Skip to content

Prepare tinywallet to serve as an out-of-process signing backend - #10

Merged
senamakel merged 29 commits into
mainfrom
tinywallet-module
Aug 11, 2026
Merged

Prepare tinywallet to serve as an out-of-process signing backend#10
senamakel merged 29 commits into
mainfrom
tinywallet-module

Conversation

@senamakel

@senamakel senamakel commented Aug 11, 2026

Copy link
Copy Markdown
Member

What

Lets a host run transaction building somewhere other than its own binary — specifically a loadable tinybus module — so the chain libraries that building requires are absent from the host entirely, while key material never leaves it.

This is the groundwork. The tinywallet-module cdylib itself follows in a second PR; nothing here depends on tinybus.

Why

OpenHuman's web3 family carries bitcoin, ethers-core, ethers-signers and coins-bip39 to sign four chains' transactions. Measured against the shipped product profile, moving signing out sheds 51 crates / 56 packages, including a native C build.

The blocker was structural: bitcoin had two parents, OpenHuman and tinywallet, so cutting the host's direct edge shed nothing — tinywallet re-supplied it through key, which required btc. Hence the first change below.

The four changes

bitcoin is no longer reachable from address validation or key derivation

It did three separable jobs, and only one justified its weight — it carries secp256k1 and therefore a native C build, which every consumer paid for even when all it wanted was to check an address.

Job Now Why
BIP-32 walk coins-bip32 (pure-Rust k256) Still delegated: a wrong derivation returns a valid key for the wrong account, silently. Already in the graph beneath coins-bip39, so it costs nothing.
Address parsing owned, over bech32 + the existing bs58 Safe to own — the opposite failure mode: a wrong parser is caught by the first vector.
PSBT build/sign keeps bitcoin, behind tx alone A full implementation genuinely earns its weight here.

key no longer implies tx, so the profile a host needs resolves with no bitcoin and no secp256k1 — 68 crates against 78. The published BTC and EVM derivation vectors still produce byte-identical addresses.

Important

This found a real regression in the swap. coins-bip32 increments BIP-32 depth unguarded: it panics in debug and wraps silently in release, deriving at the wrong depth, where bitcoin's Xpriv returned MaximumDepthExceeded. key/bip32.rs now bounds depth itself, with a test.

tinywallet::wire — the host/backend contract

Outside every chain gate and dependency-free beyond serde, so a host can take this crate with default-features = false, share one definition of the contract, and link no chain library. The same carve-out tinydocs::spec makes.

Building and signing are separable on all four chains

Each chain gains a pair — a digest (sighashes for Bitcoin, which needs one per input) and attach_signature(s).

Every existing sign() is rewritten to route through the same attach_*. Two copies of an EIP-155 v computation or a witness layout are free to drift, and the failure mode of that drift is a perfectly valid signature over the wrong transaction. Equivalence tests compare both paths byte-for-byte, including a multi-input Bitcoin spend where signature ordering matters.

tinywallet::eip712

EIP-712 typed data and the EIP-3009 authorization x402 signs, over sha3 alone — no ABI encoder, no bignum, no signer stack. Both published type hashes are pinned and re-derived from their type strings in tests, so pinning is safe rather than merely fast. This is what lets a host drop ethers-core.

Design note: keys stay with the host

SigningRequest carries no secret, and AttachRequest re-sends the transaction fields rather than a handle — so a backend keeps no state between the two calls, and needs no store, no bounds, and no expiry for callers that never come back. Building is deterministic, so rebuilding reproduces the transaction the digests were computed over.

The cost is two round trips instead of one. The alternative — shipping the mnemonic to the backend — is one round trip and sheds ~23 more crates, and was rejected: a loadable module shares the address space, but that is a reason not to widen what crosses the boundary, not a reason to stop caring.

Verification

  • 271 tests pass; clippy clean at this crate's strict bar (pedantic, unwrap_used, missing_docs) on the full, host, and bare profiles.
  • --no-default-features and --features wire both build and test standalone.
  • Two pre-existing doctests that failed under any narrow feature build are now guarded.

Compatibility

No public API is removed. Every existing sign() keeps its signature and its bytes. Chain gains conditional serde derives. The btc feature no longer pulls bitcoin — a consumer relying on that transitively must name it, which is the point.

Summary by CodeRabbit

  • New Features

    • Added EIP-712 hashing support for EIP-3009 transfer authorizations.
    • Added a standardized wire format for signing requests, signatures, and signed transactions across Bitcoin, EVM, Solana, and Tron.
    • Added split-signing workflows for generating digests/payloads and attaching signatures.
    • Added broader Bitcoin address validation and P2WPKH address derivation.
    • Added optional Serde serialization for chain values and expanded feature configuration.
  • Bug Fixes

    • Improved validation of unsupported networks, invalid signatures, derivation paths, and malformed transaction data.

Lets a host run transaction building somewhere other than its own binary —
specifically a loadable tinybus module — so the chain libraries that building
requires are absent from the host entirely, while key material never leaves it.

Four changes, in dependency order.

**`bitcoin` is no longer reachable from address validation or key derivation.**
It did three separable jobs here, and only one of them justified its weight:
it carries `secp256k1` and therefore a native C build, which every consumer
paid for even when all it wanted was to check an address.

  - BIP-32 walk (`key/bip32.rs`) now delegates to `coins-bip32`, whose backend
    is the pure-Rust `k256`. Still delegated, deliberately: a wrong derivation
    returns a valid key for the wrong account, silently. `coins-bip32` is
    already in the graph beneath `coins-bip39`, so this costs nothing.
  - Address parsing (`address/btc.rs`) is now owned directly, over `bech32` and
    the `bs58` this crate already had. Safe to own because the failure mode is
    the opposite one: a wrong parser is caught by the first vector.
  - PSBT build/sign keeps `bitcoin`, behind `tx` alone.

  `key` no longer implies `tx`, so the profile a host needs — addresses plus
  derivation — resolves with no `bitcoin` and no `secp256k1`: 68 crates against
  78. The published BTC and EVM derivation vectors still produce byte-identical
  addresses, which is the check that matters.

  This found a real regression in the swap: `coins-bip32` increments BIP-32
  depth unguarded, panicking in debug and **wrapping silently in release** —
  deriving at the wrong depth — where `bitcoin`'s `Xpriv` returned
  `MaximumDepthExceeded`. `key/bip32.rs` now bounds depth itself.

**`tinywallet::wire`** is the host/backend contract: outside every chain gate
and dependency-free beyond serde, so a host can take this crate with
`default-features = false`, share one definition, and link no chain library.
Same carve-out `tinydocs::spec` makes.

**Building and signing are separable** on all four chains. Each gains a pair —
digest/`sighashes` and `attach_signature(s)` — so a caller holding the key
elsewhere can sign the bytes and hand back only a signature. Every existing
`sign()` is rewritten to route through the same `attach_*`, because two copies
of an EIP-155 `v` computation or a witness layout are free to drift, and the
failure mode of that drift is a valid signature over the wrong transaction.
Equivalence tests compare both paths byte-for-byte, including a multi-input
Bitcoin spend where signature ordering matters.

**`tinywallet::eip712`** hashes EIP-712 typed data and the EIP-3009
authorization x402 signs, over `sha3` alone — no ABI encoder, no bignum, no
signer stack. Both published type hashes are pinned and re-derived from their
type strings in tests. This is what lets a host drop `ethers-core`.

Nothing here signs on a host's behalf and no new type carries key material.

271 tests pass; clippy is clean on the full, host, and bare profiles.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@senamakel, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 30 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 121352c3-d5db-4dc1-ace2-b2e6cf99d7c2

📥 Commits

Reviewing files that changed from the base of the PR and between 0883d5a and 22a1171.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • .github/workflows/ci.yml
  • .github/workflows/release.yml
  • Cargo.toml
  • crates/tinywallet-module/Cargo.toml
  • crates/tinywallet-module/src/lib.rs
  • crates/tinywallet-module/src/service/mod.rs
  • crates/tinywallet-module/src/service/test.rs
  • crates/tinywallet-module/tests/module_e2e.rs
  • docs/specs/tinybus-module.md
  • src/abi/mod.rs
  • src/abi/test.rs
  • src/address/btc.rs
  • src/address/btc/test.rs
  • src/lib.rs
  • vendor/tinybus
📝 Walkthrough

Walkthrough

This PR separates Bitcoin parsing and key-derivation dependencies, adds EIP-712 hashing and a serialized wire contract, and exposes split-signing APIs for Bitcoin, EVM, Solana, and Tron transactions.

Changes

Wallet signing contracts

Layer / File(s) Summary
Feature and module wiring
Cargo.toml, src/lib.rs, src/chain/mod.rs, src/address/mod.rs
Feature declarations now cover serde, eip712, and wire. The new modules are exported conditionally. Chain supports lowercase Serde representations. Documentation examples use feature guards.
Bitcoin address and key derivation
src/address/btc.rs, src/key/bip32.rs, src/key/btc.rs, src/key/evm.rs, src/key/tron.rs, src/key/test.rs
Bitcoin address parsing and P2WPKH encoding use dedicated codecs and HASH160. BIP-32 derivation uses coins-bip32 and rejects paths deeper than 255 levels.
EIP-712 hashing
src/eip712/mod.rs, src/eip712/test.rs
The module adds EIP-3009 domain and authorization hashing, decimal uint256 conversion, and 0x1901 signing-digest construction with validation tests.
Signing wire contract
src/wire/mod.rs, src/wire/test.rs
The wire contract defines signing payloads, typed signatures, public keys, transaction specifications, UTXOs, requests, and signed responses. Tests validate serialized tags, fields, and round trips.
Split transaction signing
src/tx/btc.rs, src/tx/evm.rs, src/tx/solana.rs, src/tx/tron.rs, src/tx/test.rs
Transaction modules expose digest or sighash generation and signature attachment. Tests compare split signing with one-shot signing and reject invalid keys, signature counts, and recovery IDs.

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

Possibly related PRs

Poem

A rabbit checks each hash in line,
While signatures split and recombine.
Bech32 hops through every gate,
Wire contracts serialize straight.
EIP-712 makes bytes align—
“Thump-thump!” says Bun. “The build is fine!”

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: preparing tinywallet for use as an out-of-process signing backend.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 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 and others added 14 commits August 11, 2026 12:21
The ABI decoding function now returns an error instead of panicking when given an empty byte slice, ensuring robust handling of malformed or incomplete input data.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `sha3` crate is now imported only inside the test-scoped `keccak` function, removing the top-level import and the workaround that silenced the unused-import warning in non-test builds. This makes the dependency on the hashing library explicit for test code only, since production uses the precomputed `TRANSFER_SELECTOR` constant.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the test in `src/abi/test.rs` to properly verify that the ABI module returns an error when given an empty input, rather than silently succeeding. This ensures the validation logic is correctly tested and prevents false positives in the test suite.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a new `abi` feature that exposes the `tinywallet::abi` module for building ERC-20 `transfer` calldata. This feature is placed outside the `tx` gate because calldata construction is an input to transaction building, allowing hosts that build transactions elsewhere to use it without pulling in the full transaction dependency chain.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a workspace section to Cargo.toml that includes the tinywallet-module crate as a member while explicitly excluding the vendored tinybus submodule to prevent workspace conflicts. The tinybus submodule is also updated to a newer commit.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Bump the serde crate version from 1.0.197 to 1.0.200 in the tinywallet-module Cargo.toml to incorporate the latest bug fixes and improvements.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When a user submits a wallet creation request with an empty name, the service now returns a validation error instead of proceeding with an invalid state. This prevents potential downstream issues and provides clearer feedback to the caller.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed an unused import from the tinywallet module's lib.rs file to eliminate a compiler warning about dead code. This cleanup keeps the codebase tidy without affecting any functionality.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new test module to the service layer to enable unit testing of service functions. This establishes the testing infrastructure needed to verify service behavior and catch regressions.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the tinybus event-bus library, its macros crate, the tinybus-module abstraction, and the tinywallet-module crate to the workspace, along with all their transitive dependencies. This change also moves lint configuration from per-crate to workspace-level inheritance so that every member crate is held to the same standard.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ds and handle non-exhaustive en

Adds the `bs58` crate to encode 64-byte Solana signatures directly, since the existing `address::solana::encode` only handles 32-byte addresses. Also adds wildcard arms to match statements on `TransactionSpec` and `Signature` enums, which are marked `#[non_exhaustive]`, so that future variants produce a clear error instead of a compilation failure or silent misbehavior. Fixes a type mismatch in EVM transaction construction by converting `nonce` and `gas_limit` from their source types to `u128`.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The test now checks that the service correctly rejects invalid inputs by asserting the expected error response, ensuring the validation change is properly covered.

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

The tinywallet-module crate now depends on the bitcoin library with the secp-recovery feature enabled, because the service tests need to sign messages using the recoverable secp256k1 API exactly as the host would, ensuring the tests exercise the real cryptographic boundary rather than a simplified helper.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformat several long lines in the decimal_u128 function and test files to comply with the project's line-length conventions, wrapping them across multiple lines for improved readability without any change in behaviour.

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

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🧹 Nitpick comments (7)
src/tx/evm.rs (1)

159-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

recovery_as_u64 cannot fail here.

recovery_id is a u8, so i32::from(recovery_id) is never negative and the negative-id branch is unreachable. u64::from(recovery_id) states the same thing without the fallible round trip.

♻️ Proposed simplification
-        let recovery = recovery_as_u64(i32::from(recovery_id))?;
-        let v = checked_v(recovery, self.chain_id)?;
+        let v = checked_v(u64::from(recovery_id), self.chain_id)?;

Keep recovery_as_u64 if another caller still passes a raw i32.

🤖 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/tx/evm.rs` around lines 159 - 160, In the code around recovery_id,
replace the fallible recovery_as_u64(i32::from(recovery_id)) conversion with a
direct u64 conversion from the u8 value, removing the unnecessary error
propagation. Retain recovery_as_u64 only if another caller still requires
conversion from a raw i32.
src/tx/solana.rs (1)

370-381: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The assertion does not test what the name states.

message.len() > 32 holds for any Solana message. It does not show that the signed payload is the message rather than a digest, and it does not exercise the pre-hashing mistake the comment describes.

Sign a digest of the message and assert the result does not verify against from. That failure is the property worth pinning.

💚 Proposed test change
     let transfer = transfer();
     let message = transfer.message().unwrap();
-    assert!(
-        message.len() > 32,
-        "a Solana message is the full serialized transaction, not a 32-byte digest"
-    );
+    assert!(message.len() > 32, "a message is not a 32-byte digest");
+
+    use ed25519_dalek::{Signer as _, SigningKey, Signature, Verifier as _, VerifyingKey};
+    use sha2::{Digest as _, Sha256};
+
+    let bytes: [u8; 32] = key().as_slice().try_into().unwrap();
+    let signing = SigningKey::from_bytes(&bytes);
+    let digest: [u8; 32] = Sha256::digest(&message).into();
+    let wrong = signing.sign(&digest).to_bytes();
+
+    let public = VerifyingKey::from_bytes(&crate::address::solana::decode(FROM).unwrap()).unwrap();
+    assert!(
+        public.verify(&message, &Signature::from_bytes(&wrong)).is_err(),
+        "signing a digest must not produce a signature the network accepts"
+    );

sha2 is already a tx dependency, so no manifest change is needed.

🤖 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/tx/solana.rs` around lines 370 - 381, Update the test
the_signed_payload_is_the_message_itself_not_a_digest to hash the serialized
message with the existing sha2 dependency, sign that digest using the transfer’s
signing path, and assert the resulting signature does not verify against from.
Remove the message-length assertion while preserving the test’s focus on
rejecting pre-hashed payloads.
src/tx/tron.rs (2)

120-135: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Consider normalizing s here, as the Bitcoin path does.

Transfer::attach_signatures in src/tx/btc.rs calls normalize_s at Line 323, with the reasoning that a host signing through a library that does not normalize still produces a broadcastable transaction. attach_signature here, and the EVM equivalent, place that burden on the host instead. src/wire/mod.rs Lines 65-77 document the requirement, so the contract is stated, but the enforcement is asymmetric across the three secp256k1 chains.

Either normalize in all three attachment paths or record in this doc comment why Bitcoin is the exception.

🤖 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/tx/tron.rs` around lines 120 - 135, Normalize the signature’s s component
in attach_signature before constructing the 65-byte Tron output, matching the
normalize_s behavior used by Bitcoin and the other secp256k1 attachment paths.
Preserve the recovery_id validation and output layout, and ensure the doc
comment reflects the enforced normalization contract if needed.

115-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

recompute_txid can now be expressed through digest.

Lines 44-47 repeat the same decode and SHA-256. The doc comment states the two are the same value by construction; routing one through the other makes that structural instead of relying on the test at src/tx/test.rs Line 366.

♻️ Proposed change to `recompute_txid`
 pub fn recompute_txid(raw_data_hex: &str) -> Result<String> {
-    let raw = decode_hex(raw_data_hex)?;
-    Ok(hex_lower(&Sha256::digest(&raw)))
+    Ok(hex_lower(&digest(raw_data_hex)?))
 }
🤖 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/tx/tron.rs` around lines 115 - 118, Update recompute_txid to delegate to
the existing digest function instead of independently decoding raw data and
computing SHA-256. Preserve recompute_txid’s current return behavior while
making the shared calculation structurally explicit.
src/tx/btc.rs (3)

622-638: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

wrong likely fails the parse guard, not the address-mismatch guard.

[0x02u8; 33] is a prefix byte plus an x-coordinate that is almost certainly not on secp256k1. CompressedPublicKey::from_slice rejects it, so check_controls_from returns before the address comparison. The test passes without covering the branch it targets.

Derive a real key from another path, as a_key_that_does_not_control_the_sender_is_rejected does at Line 496, and use its compressed public key.

💚 Proposed test change
-        let wrong = [0x02u8; 33];
+        let other = crate::key::derive(crate::Chain::Btc, VECTOR, "m/84'/0'/0'/0/1")
+            .unwrap()
+            .secret_bytes()
+            .to_vec();
+        let wrong = {
+            use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+            let secret = SecretKey::from_slice(&other).unwrap();
+            PublicKey::from_secret_key(&Secp256k1::new(), &secret).serialize()
+        };
🤖 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/tx/btc.rs` around lines 622 - 638, Update
a_public_key_that_does_not_control_the_sender_is_refused to derive a valid
compressed public key from a different key path, following the setup used by
a_key_that_does_not_control_the_sender_is_rejected. Replace the hard-coded wrong
value while preserving both sighashes and attach_signatures assertions so they
exercise the sender-address mismatch branch and return Error::Signing.

313-331: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

The split attachment paths accept backend signatures without confirming they sign what this crate built. Each one-shot sign derives the key locally and refuses a key that does not control the sender. The matching attachment method assembles whatever bytes it is handed. The out-of-process backend this PR prepares for is exactly the caller that can get this wrong, and the result is a well-formed transaction the network rejects after the fee is committed.

  • src/tx/btc.rs#L313-L331: verify each compact signature against its corresponding BIP-143 sighash and public_key before building the witness, so a transposed or foreign signature fails locally.
  • src/tx/solana.rs#L137-L153: verify the supplied 64-byte signature against the ed25519 key decoded from self.from, restoring the guard sign applies at Line 124.
🤖 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/tx/btc.rs` around lines 313 - 331, The split attachment paths must
validate backend signatures before attaching them. In src/tx/btc.rs:313-331,
update the attachment loop around the witness construction to verify each
compact signature against its corresponding BIP-143 sighash and public_key,
rejecting transposed or foreign signatures before building the witness. In
src/tx/solana.rs:137-153, verify the supplied 64-byte signature against the
Ed25519 public key decoded from self.from, restoring the validation performed by
sign; both sites should fail locally on invalid signatures.

221-236: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

sign now builds the transaction twice.

sighashes calls build, and attach_signatures calls build again. Each build clones and sorts the whole UTXO set in select_coins. The result is correct because build is deterministic, but the work doubles for a wallet with many UTXOs.

Consider adding a private helper that takes an already-built (Transaction, Selection) and is shared by both public methods, leaving the public API unchanged.

🤖 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/tx/btc.rs` around lines 221 - 236, Avoid rebuilding the transaction in
sign by introducing a private helper that accepts the already-built
(Transaction, Selection) result and performs signature attachment. Update
sighashes and attach_signatures to reuse this helper or shared build output,
while keeping their public APIs and transaction ordering behavior unchanged.
🤖 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 `@Cargo.toml`:
- Around line 105-108: Update the serde feature definition in Cargo.toml to also
enable serde’s derive feature, while preserving the existing dep:serde
dependency activation so Chain’s serde derives compile when the serde feature is
enabled.

In `@src/address/btc.rs`:
- Around line 207-246: Update the HRP validation in parse_bech32 to compare
case-insensitively, using bech32::hrp::BC or an equivalent ASCII-insensitive
comparison instead of direct string inequality. Preserve rejection of
non-mainnet HRPs while accepting valid uppercase and lowercase mainnet
addresses.

In `@src/wire/mod.rs`:
- Around line 120-145: Ensure SigningRequest and AttachRequest reject
inconsistent chain and transaction.kind combinations during request validation,
or remove the redundant chain field while preserving the API contract. Apply the
check to both request types and add rejection tests covering mismatched values
for each.

---

Nitpick comments:
In `@src/tx/btc.rs`:
- Around line 622-638: Update
a_public_key_that_does_not_control_the_sender_is_refused to derive a valid
compressed public key from a different key path, following the setup used by
a_key_that_does_not_control_the_sender_is_rejected. Replace the hard-coded wrong
value while preserving both sighashes and attach_signatures assertions so they
exercise the sender-address mismatch branch and return Error::Signing.
- Around line 313-331: The split attachment paths must validate backend
signatures before attaching them. In src/tx/btc.rs:313-331, update the
attachment loop around the witness construction to verify each compact signature
against its corresponding BIP-143 sighash and public_key, rejecting transposed
or foreign signatures before building the witness. In src/tx/solana.rs:137-153,
verify the supplied 64-byte signature against the Ed25519 public key decoded
from self.from, restoring the validation performed by sign; both sites should
fail locally on invalid signatures.
- Around line 221-236: Avoid rebuilding the transaction in sign by introducing a
private helper that accepts the already-built (Transaction, Selection) result
and performs signature attachment. Update sighashes and attach_signatures to
reuse this helper or shared build output, while keeping their public APIs and
transaction ordering behavior unchanged.

In `@src/tx/evm.rs`:
- Around line 159-160: In the code around recovery_id, replace the fallible
recovery_as_u64(i32::from(recovery_id)) conversion with a direct u64 conversion
from the u8 value, removing the unnecessary error propagation. Retain
recovery_as_u64 only if another caller still requires conversion from a raw i32.

In `@src/tx/solana.rs`:
- Around line 370-381: Update the test
the_signed_payload_is_the_message_itself_not_a_digest to hash the serialized
message with the existing sha2 dependency, sign that digest using the transfer’s
signing path, and assert the resulting signature does not verify against from.
Remove the message-length assertion while preserving the test’s focus on
rejecting pre-hashed payloads.

In `@src/tx/tron.rs`:
- Around line 120-135: Normalize the signature’s s component in attach_signature
before constructing the 65-byte Tron output, matching the normalize_s behavior
used by Bitcoin and the other secp256k1 attachment paths. Preserve the
recovery_id validation and output layout, and ensure the doc comment reflects
the enforced normalization contract if needed.
- Around line 115-118: Update recompute_txid to delegate to the existing digest
function instead of independently decoding raw data and computing SHA-256.
Preserve recompute_txid’s current return behavior while making the shared
calculation structurally explicit.
🪄 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: a1bdeb89-0bfb-4e77-864e-2196a362f3d9

📥 Commits

Reviewing files that changed from the base of the PR and between 537340e and 0883d5a.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • Cargo.toml
  • src/address/btc.rs
  • src/address/mod.rs
  • src/chain/mod.rs
  • src/eip712/mod.rs
  • src/eip712/test.rs
  • src/key/bip32.rs
  • src/key/btc.rs
  • src/key/evm.rs
  • src/key/test.rs
  • src/key/tron.rs
  • src/lib.rs
  • src/tx/btc.rs
  • src/tx/evm.rs
  • src/tx/solana.rs
  • src/tx/test.rs
  • src/tx/tron.rs
  • src/wire/mod.rs
  • src/wire/test.rs

Comment thread Cargo.toml
Comment thread src/address/btc.rs
Comment thread src/wire/mod.rs
senamakel and others added 9 commits August 11, 2026 12:28
Changed the `build_failed` function to take a reference to `tx::Error` instead of an owned value, and updated all call sites to pass `&e` instead of `e`. This avoids unnecessary cloning of error values when mapping errors in the transaction building pipeline.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The end-to-end test for the tinywallet module was accidentally removed during a previous refactor. This change restores the test to ensure the module's core functionality is validated in integration scenarios.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the ed25519-dalek and tokio crates as dependencies for the tinywallet-module crate. The ed25519-dalek library is needed to sign Solana transfers using ed25519 keys in end-to-end tests, while tokio provides the async runtime required to drive a real broker and loader during those tests.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the initial specification document for the tinybus module, defining its interface and behavior to guide implementation and future development.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a new workflow stage that builds native module bundles for eleven platform targets and creates a GitHub release with checksum verification. This enables automated distribution of compiled modules across Linux, macOS, and Windows architectures.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a new end-to-end test job that builds the TinyBus loadable module as a cdylib and runs the module_e2e test, which validates that signing through the dynamic loader matches in-process signing byte for byte. Fix the release workflow to also bump the version in the module crate's Cargo.toml and update the workspace lockfile, preventing a mismatch where the archive name would reflect the root crate version while the library inside reports an older version.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Vendored submodules contain third-party code whose coverage should not be enforced by this repository. The filter now skips files under the vendor directory to prevent false positives in the coverage gate.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds thirteen test cases that exercise every rejection path in the parser that was introduced when this module stopped delegating to the `bitcoin` crate. Each test targets a specific rule from BIP-173 or BIP-350, using the published test vectors where available, to ensure that a rejection that never fires is distinguishable from one that is wrong.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a unit test that exercises the defensive error paths in `decode_evm_address` with a short hex string and a non-hex character, verifying they return an error instead of panicking. The happy path with an unprefixed address is also tested to confirm the `0x` prefix is optional.

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

Copy link
Copy Markdown
Member Author

Added: the loadable module and its release pipeline

Pushed the second half. The PR now covers the whole tinywallet side of the port.

crates/tinywallet-module — the cdylib

Serves ai.tinyhumans.tinywallet.Wallet with two methods, BuildUnsigned and AttachSignature. Neither accepts key material, which is the point of the two-call split.

Notably smaller than the tinydocs module, because everything travels inline: a tinybus frame is JSON capped at 16 MiB where a byte array costs ~3.5 bytes per byte — a real constraint for a generated .docx, and irrelevant for a transaction. The largest payload is a Bitcoin spend's UTXO list. So there are no streams, no chunking, and no output store, and none of the bounds/expiry apparatus those require.

The workspace's [lints] moved to [workspace.lints] so the module crate is held to exactly the same bar as the library.

The test that matters

crates/tinywallet-module/tests/module_e2e.rs loads the built cdylib through the real loader and broker — ABI descriptor, manifest admission, frames on the wire — and asserts a transaction signed through the module is byte-for-byte the library's own output, on EVM, a multi-input Bitcoin spend, and Solana. It passes:

test the_built_module_signs_every_chain_over_a_real_broker ... ok

That equivalence is the claim the module rests on; everything else here would keep passing if the artifact stopped loading. It runs in CI on every push (module-e2e job) against a freshly built artifact.

Release pipeline

Ported from tinydocs' proven workflow: 11 per-platform bundles (ubuntu 22.04/24.04, macOS 15/26, Windows 2022/2025/11, x86_64 and arm64), each carrying a modules.toml digest, aggregated into a release checksum.toml by tinybus itself, then verified by actually downloading and loading the published artifact.

Two corrections to the ported version, both bugs in the original:

  1. The version bump now covers both crates. It only touched the root, so the module's cdylib would ship in an archive named tinywallet-module-0.2.0-*.tar.gz while reporting 0.1.0 internally, with nothing to catch it. Uses cargo update --workspace because cargo update -p cannot bump a path dependency, and the later --locked steps fail against a stale lock.
  2. The coverage gate excluded vendored code. Adding the module crate put vendor/tinybus into the build graph, so tinybus-module/src/lib.rs appeared in the report at 0% and failed the 90% per-file gate. Somebody else's code, not this repository's to enforce.

Also fixed while there

The gate caught genuine gaps in my own new code, and closing them found the parser's subtle cases were untested. address/btc.rs now has vectors for the BIP-350 rule specifically — a v0 address carrying a bech32m checksum, and a taproot address carrying a bech32 one, both correctly rejected — plus taproot as a valid recipient, wrong-network reporting for testnet/regtest in both encodings, mixed case, and a wrong-length v0 program. The abi decoder's defensive arms are tested directly, since the public path validates first and cannot reach them.

Verification

  • 302 tests, all passing, including the real-loader E2E.
  • Clippy clean workspace-wide at the strict bar (pedantic, unwrap_used, missing_docs).
  • Zero files below the 90% per-file coverage gate.
  • docs/specs/tinybus-module.md documents the interface, both signing schemes and the prehash/no-prehash distinction a host must not get wrong.

Next: openhuman migrates onto this and drops bitcoin, ethers-core, ethers-signers — the measured 51 crates / 56 packages.

senamakel and others added 3 commits August 11, 2026 12:39
The MSRV job was building all workspace crates, but `tinywallet-module` is unpublished and requires a newer compiler due to its dependency on `tinybus`. Scoping the build to `tinywallet` prevents the module from forcing an unnecessarily high MSRV on the library's consumers. The module's documentation comment is also updated to point to the external spec file and clarify how to view the private service module's docs.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reworded the inline comment in the coverage threshold script to better explain why vendored submodule files are excluded from the coverage check, and added a note about the single-quoting constraint to prevent future confusion.

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

The address parser now dispatches on the structural shape of a bech32 string rather than matching against a hardcoded list of known human-readable parts. This allows any bech32-shaped address, including those from test networks or other chains, to be routed to the bech32 parser and reported as a wrong-network error instead of being misidentified as malformed base58. The change also replaces the manual `Hrp::parse` call in `encode_p2wpkh` with the constant `bech32::hrp::BC`, removing an unreachable error path.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
senamakel and others added 2 commits August 11, 2026 12:47
Adds five test cases that exercise edge cases in Bitcoin address validation: unrecognised version bytes, wrong hash lengths, empty payloads, testnet P2SH addresses, and a Litecoin bech32 address that should produce a WrongNetwork error rather than a misleading base58 parse failure.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Reformatted the error mapping closure in `encode_p2wpkh` to use a single expression instead of a block, and removed unnecessary line breaks in the test for empty base58check payloads. These changes improve code consistency without altering behavior.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@senamakel
senamakel merged commit 92c5b8a into main Aug 11, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant