Extract document and wallet logic into vendored tinydocs + tinywallet - #5476
Conversation
Adds the tinydocs and tinywallet repositories as submodules under the vendor directory, registering them in .gitmodules so they can be pulled in alongside the existing tinybus dependency. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new document types module under the tools implementation to support structured document handling, along with the tinydocs vendor dependency required for its functionality. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Integrate the tinydocs library as a vendored dependency and implement the document engine within the openhuman tools framework, enabling document processing capabilities for the tooling system. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce the document types module in the openhuman tools implementation, providing the core type definitions needed for document handling. This change also includes the tinydocs vendor dependency to support the new functionality. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The document types module now checks for the presence of the tinydocs vendor directory before attempting to use it, preventing a panic when the directory is absent. This ensures graceful fallback behavior during development or incomplete setups. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replace inline Bitcoin, Solana, and Tron address parsing with calls to the vendored tinywallet crate, which now owns the address format rules. This removes duplicated logic and keeps OpenHuman-specific code focused on the Result<String, String> shape the domain expects. The Tron hex conversion now validates before decoding, catching malformed addresses earlier. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds a test module that probes the `validate_evm_address` function with a variety of address formats, including mixed case, missing prefix, and invalid characters, to verify the validator's behaviour across edge cases. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the tinywallet crate as an optional path dependency behind the default-on `web3` feature. This crate provides host-agnostic parsing, validation, and encoding conversions for Bitcoin, EVM, Solana, and Tron addresses, keeping those rules out of the OpenHuman-specific codebase. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the tinywallet crate as a dependency under the web3 feature flag, enabling wallet functionality for web3 operations. The new dependency provides Bitcoin address generation and transaction handling capabilities through its bitcoin, bs58, hex, and thiserror dependencies. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Replaced the inline EVM address validation and the per-chain address dispatch in execution.rs with calls to the vendored tinywallet crate, which now owns the address format logic for all four supported chains. This removes the duplicated ethers-core dependency and keeps the chain-to-tinywallet mapping centralized in the validate_address function. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…nydocs,vendor/tinywallet Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…sign rules Add a new section to AGENTS.md describing the two vendored crates (tinydocs and tinywallet) that now own logic previously in this repo, explaining the split criterion, each crate's responsibilities, and the design consequences that developers should know before touching either seam. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Update the pinned commits for the tinydocs and tinywallet vendor submodules to their latest versions. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughThe project now vendors TinyDocs and TinyWallet. Document generation, validation, truncation, wallet address validation, and wallet key derivation use the vendored crates. OpenHuman also provides TinyWallet transport integration. ChangesVendored integrations
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@AGENTS.md`:
- Around line 210-212: Verify the uppercase 0X behavior in tinywallet and the
prior ethers-core path using the referenced parsing symbols, then update the
contradictory AGENTS.md statement to describe the actual compatibility result.
Keep the documented behavior precise and do not change implementation code or
broaden prefix acceptance.
In `@src/openhuman/tools/impl/document/types.rs`:
- Around line 123-124: Update validate_input to emit stable [document:types]
diagnostics for validation entry and completion/failure, including bounded
metadata such as title_chars, section_count, and error kind. Do not log document
text, author values, or error details; preserve the existing DocumentError
conversion and validation behavior.
In `@src/openhuman/web3/wallet/execution.rs`:
- Around line 345-352: Add stable non-sensitive [domain]-prefixed diagnostics to
the TinyWallet validation flows: in
src/openhuman/web3/wallet/execution.rs:345-352 log chain dispatch and
accepted/rejected validation results, propagating any available request or quote
correlation field; in src/openhuman/web3/wallet/chains/btc.rs:74-86 log
recipient/sender role and result; in
src/openhuman/web3/wallet/chains/evm.rs:344-345 and
src/openhuman/web3/wallet/chains/solana.rs:69-70 log validation results; and in
src/openhuman/web3/wallet/chains/tron.rs:37-50 log validation and conversion
results. Use the relevant visible validation functions and existing logging
conventions, and never include addr or untrusted error text.
🪄 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: 7410ae06-ddda-4c68-984a-37bf5ee785db
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.gitmodulesAGENTS.mdCargo.tomlsrc/openhuman/tools/impl/document/engine.rssrc/openhuman/tools/impl/document/types.rssrc/openhuman/web3/wallet/chains/btc.rssrc/openhuman/web3/wallet/chains/evm.rssrc/openhuman/web3/wallet/chains/solana.rssrc/openhuman/web3/wallet/chains/tron.rssrc/openhuman/web3/wallet/execution.rsvendor/tinydocsvendor/tinywallet
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0103 · 131,280 in / 32,356 out · 104,538 cached (80%) · z-ai/glm-5.2
critique: $0.0044 · 46,576 in / 14,946 out · 37,696 cached (81%) · z-ai/glm-5.2
security: $0.0029 · 32,482 in / 10,010 out · 26,807 cached (83%) · z-ai/glm-5.2
tests: $0.0016 · 17,191 in / 5,247 out · 12,444 cached (72%) · z-ai/glm-5.2
description: $0.0008 · 18,179 in / 1,455 out · 14,510 cached (80%) · z-ai/glm-5.2
What this change touches10 files, +342 -588 across 4 components. The code graph knows nothing about these files yet — normal for newly added files, and a cold index otherwise. flowchart LR
n0["src/openhuman/tools/impl/document<br/>2 files +194 -503<br/>1 finding"]:::flagged
n1["src/openhuman/web3/wallet/chains<br/>4 files +50 -61<br/>4 findings"]:::flagged
n2["root<br/>3 files +78 -8"]:::changed
n3["src/openhuman/web3/wallet<br/>1 file +20 -16<br/>1 finding"]:::flagged
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
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the tinywallet submodule to a new commit and added several new dependencies to Cargo.lock, including coins-bip39, ed25519-dalek, hmac, sha2, sha3, and zeroize. Removed unused imports for hmac and Sha512 from the solana module to clean up the code. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…llet Update the pinned commits for the tinydocs and tinywallet vendor submodules to incorporate upstream changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…et crate Replace the duplicated BIP-32 key derivation logic in both the BTC and Tron wallet modules with calls to the vendored tinywallet crate, which now owns the secp256k1 derivation and address construction. This removes over 100 lines of hand-rolled HMAC-SHA512 walking, path parsing, and Keccak address computation, centralizing the logic in a single tested implementation while keeping mnemonic custody in this crate. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Remove unused imports from the Bitcoin and Tron chain modules to eliminate compiler warnings and clean up the codebase. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a check to return an error when no wallet transport is configured, preventing a panic or undefined behavior when the transport is accessed but not set. This improves robustness by ensuring a clear failure mode instead of relying on the caller to verify the transport state. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The tinywallet dependency in Cargo.toml now includes the "net", "asset", "client", and "tx" features, and the corresponding async-trait, serde, and serde_json dependencies have been added to Cargo.lock to support these new capabilities. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
A new `transport` module is introduced to host the endpoint resolution, failover, and redaction logic that belongs on the config side of tinywallet's `Transport` seam, keeping these concerns separate from the RPC layer. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/web3/wallet/chains/btc.rs (1)
452-457: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winTighten the wrong-network assertion.
The assertion accepts
"not on mainnet"or"invalid". Any generic parse failure now satisfies the test. The test no longer proves thattinywalletreports the wrong-network condition distinctly. Assert the wrong-network wording only.💚 Proposed test tightening
- // `tinywallet` reports a wrong-network address as a distinct condition - // from a malformed one, so the message names the required network. - assert!( - err.contains("not on mainnet") || err.contains("invalid"), - "got: {err}" - ); + // `tinywallet` reports a wrong-network address as a distinct condition + // from a malformed one, so the message names the required network. + assert!(err.contains("not on mainnet"), "got: {err}");🤖 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/web3/wallet/chains/btc.rs` around lines 452 - 457, Update the assertion in the wrong-network wallet test to require only the specific “not on mainnet” wording, removing the generic “invalid” alternative while preserving the existing error output in the failure message.
🧹 Nitpick comments (1)
src/openhuman/web3/wallet/transport.rs (1)
107-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd structured logging to the new transport flow.
The three
Transportmethods perform external calls and classify errors. Neither method logs anything. The classification branch at Lines 98-105 decides whether a caller retries, and no log records that decision. A production failover problem leaves no trace.Add
debug!entries with a stable, grep-friendly prefix for the resolved network, the method or path, and the classification outcome. Log the redacted endpoint only.As per coding guidelines: "New or changed flows should log entry/exit, branches, external calls, retries/timeouts, state transitions, and errors using stable, grep-friendly prefixes."
🤖 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/web3/wallet/transport.rs` around lines 107 - 138, Update the Transport implementation methods json_rpc, rest_get, and rest_post to add debug! logs around each external call, using stable grep-friendly prefixes that include the resolved network, method or path, and classification outcome. Log only redacted endpoint information, and include the classify decision so retry/failover behavior is observable without exposing sensitive data.Source: Coding guidelines
🤖 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/mod.rs`:
- Around line 37-40: Gate the wallet transport submodule declaration in `wallet`
with `#[cfg(feature = "web3")]` so `transport` is compiled only when the `web3`
feature is enabled. Leave the stub facade and its existing signature unchanged
for feature-disabled builds.
In `@src/openhuman/web3/wallet/transport.rs`:
- Around line 61-63: Update the EVM branch in resolve so a missing
network.evm_chain_id returns an error, matching the existing unknown-chain
handling instead of falling back to rpc_url_for_chain(WalletChain::Evm). Add a
test covering NetworkId::chain(tinywallet::Chain::Evm) and assert that resolve
returns an error.
In `@vendor/tinywallet`:
- Line 1: Update the vendored tinywallet submodule reference to a
repository-reachable commit containing the tinywallet#1 merge target, or publish
the currently referenced commit to a configured remote before release; preserve
the existing Cargo.toml path dependency and web3 features.
---
Outside diff comments:
In `@src/openhuman/web3/wallet/chains/btc.rs`:
- Around line 452-457: Update the assertion in the wrong-network wallet test to
require only the specific “not on mainnet” wording, removing the generic
“invalid” alternative while preserving the existing error output in the failure
message.
---
Nitpick comments:
In `@src/openhuman/web3/wallet/transport.rs`:
- Around line 107-138: Update the Transport implementation methods json_rpc,
rest_get, and rest_post to add debug! logs around each external call, using
stable grep-friendly prefixes that include the resolved network, method or path,
and classification outcome. Log only redacted endpoint information, and include
the classify decision so retry/failover behavior is observable without exposing
sensitive data.
🪄 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: bf38d14d-e2b6-425a-b828-0f621f91fc75
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlsrc/openhuman/web3/wallet/chains/btc.rssrc/openhuman/web3/wallet/chains/evm.rssrc/openhuman/web3/wallet/chains/solana.rssrc/openhuman/web3/wallet/chains/tron.rssrc/openhuman/web3/wallet/mod.rssrc/openhuman/web3/wallet/transport.rsvendor/tinydocsvendor/tinywallet
🚧 Files skipped from review as they are similar to previous changes (2)
- Cargo.toml
- vendor/tinydocs
…ld code path The documentation for tinywallet's EVM prefix handling now explicitly states that the old code path through `ethers_core` also rejected an uppercase `0X` prefix, citing the specific `fixed-hash` implementation that strips only a lowercase `0x`. This makes it clear the behaviour is unchanged from the previous implementation and should not be relaxed. Auto-committed-on: dragonfly
Added debug logging across all chain-specific address validators, the dispatch function in execution.rs, and the transport layer's RPC calls. The logs capture the validation outcome and, for the transport, the endpoint URL (redacted) and whether an error is retryable. This makes it possible to trace address rejection and RPC failures in production without changing the public API. Also fixed a test assertion in btc.rs that was too broad, added a test for Tron address length rejection, and gated the transport module behind the `web3` feature flag. Auto-committed-on: dragonfly
Reformatted multi-line debug macro invocations across the wallet transport layer and chain-specific address validation functions to use explicit line breaks and indentation, improving code readability without changing any runtime behavior. Also reordered a module declaration in `mod.rs` to keep the doc comment adjacent to its item. Auto-committed-on: dragonfly
Vendors
tinydocsandtinywalletasvendor/submodules and moves host-agnostic logic out of this repo into them.Both crates are merged to their
main, and the gitlinks here point at those merge commits — so this no longer depends on anything unmerged.The rule this split follows
A crate owns what is the same for every host; the host owns what depends on its own runtime, config, or threat model.
tinydocs.docxspec types, size limits, validation, OOXML synthesisspawn_blockinghop, generation deadlinetinywalletWhat is live in this PR
tinydocs. The agent-facing JSON tool schema is unchanged —GenerateDocumentInputis the crate'sDocumentSpecre-exported under its historical name, pinned bythe_json_wire_shape_is_unchanged_by_the_extraction.tinywallet::key, deleting ~110 lines of hand-rolled BIP-32 and SLIP-0010 fromweb3/wallet/chains/.coins_bip39andMnemonicBuilderare gone fromweb3/wallet/entirely.Transportadapter (web3/wallet/transport.rs) — the host side oftinywallet's network seam, sotinywallet::clientandtinywallet::txare consumable. Endpoint resolution, theOPENHUMAN_WALLET_RPC_<CHAIN>overrides, tiny.place Solana failover, URL redaction and the shared reqwest client all stay here and are reused unchanged.143 web3 tests plus 26 document-tool tests pass;
--no-default-featuresclean; the Feature Forwarding Gate passes.Behaviour changes — three, all deliberate
tron_address_to_hexis stricter. The old implementation decoded without a length check, so a malformed address that base58check-decoded to the wrong length produced a short hex string and failed later at the TronGrid call. It validates first now.tinywalletreports a wrong-network address as distinct from a malformed one ("not on mainnet"), and names the failing role for a non-P2WPKH sender. Agent-facing strings, not a wire contract; the two loose assertions that pinned the old wording are updated.Everything else is parity, verified rather than assumed: my first
tinywalletEVM validator accepted an uppercase0Xprefix, and I probed the realethers-corepath before swapping rather than trusting the diagnosis. It rejects0X, so the crate was tightened to match. Widening validation on a wallet path is the wrong direction to drift.Reviewer note on the Transport adapter
tinywalletsplits transport failures into retryable and authoritative because a host's failover depends on it — retrying an authoritative "insufficient funds" gets the same answer, while retrying an ambiguous failure risks a double broadcast. This repo's RPC helpers flatten both intoString, so the adapter cannot recover the distinction perfectly and classifies conservatively: anything it cannot prove is a transport failure is reported as authoritative, so an unclassifiable error stops a failover rather than driving one. A missed retry costs a request; a wrong retry can cost a duplicate transaction.Not in this PR
The signing and broadcast paths in
web3/wallet/chains/are still the live ones.tinywallet::txandtinywallet::clientare merged, tested and now reachable through the adapter, but no call site consumes them yet — repointing those is follow-up work, as is porting x402's payment-flow logic (its wire types are in review as tinywallet#4).This PR is therefore a strict subset: the seams are in place and proven against real call sites for derivation, with nothing half-migrated.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation