feat(tx): parse Tron raw_data to verify it, and split tx-codec out of tx

[senamakel]

Stacked on #16. Review that first; base retargets to main when it merges.

The gap

Tron inverts the usual split — the node builds the transaction and the client signs what it is handed. verify_transfer is the check that stands between a compromised endpoint and a signature, and today it does this:

if !raw_data_hex.to_ascii_lowercase().contains(&to_hex.to_ascii_lowercase())

A substring scan over the hex. Two things that lets through:

1. The address appearing somewhere is not the same as it being to_address. A node can build a transfer paying someone else and leave the requested address in an unrelated field. The scan is satisfied.
2. The amount is never checked. A node that builds the right recipient with the wrong value passes unconditionally.

Both are now regression tests, and both pass verify_transfer before failing the new check:

assert!(verify_transfer(&raw, TO, &id).is_ok(), "the weak check is fooled by the decoy");
// ...
match verify_contract(&raw, TO, &id, &transfer).unwrap_err() {
    Error::UntrustedResponse { reason } => assert!(reason.contains("does not pay the requested recipient")),

The change

tx::proto — a ~120-line structural protobuf reader. Not a schema compiler and not prost: it recovers field numbers and raw values over a message whose shape is already known, borrows throughout (Value::Bytes points into the caller's buffer), and leaves the meaning of field 11 to tx::tron. No new dependency — it walks &[u8].

Every accessor is singular and refuses a repeated field. The spec does permit repetition, but "last one wins" is exactly how a second recipient gets smuggled past a checker that reads the first, so a repeated singular field is treated as the attack it would be rather than a value to disambiguate.

tx::tron::verify_contract — checks contract type, the recipient at its declared field number, the amount, and for TRC-20 the full calldata including selector, call_value (a token transfer moves no TRX, so non-zero means native funds leaving alongside it) and fee_limit.

verify_transfer is kept for its existing callers in client::tron and the module service, and is now documented as the weaker check with a pointer to this one.

Provenance

Ported from the equivalent verifier in tinyhumansai/openhuman's web3/wallet/chains/tron.rs. That host had independently written the stronger check; this brings it where every host can reach it, and openhuman deletes its copy in the companion PR (tinyhumansai/openhuman#5533 and its follow-up).

Verification

Check	Result
cargo test --all-features	297 + 7 + 9 + 18 doctests, 0 failed
cargo clippy --all-features --all-targets	clean (workspace lints, -D warnings)
cargo fmt	clean
new tx::proto tests	12
new tx::tron tests	10

Coverage on the parser is adversarial rather than happy-path: repeated singular fields, wrong wire type, field number zero, unsupported wire types 3/4/6/7, truncation at each stage, a varint overrunning 64 bits, and fixed-width fields skipped without desynchronising the stream.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e3bb025e-9cd7-43c5-baa4-4129b4ef23cf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[tinysweeper]

tinysweeper found nothing blocking. Approving.

             $0.0742 · 79,892 in / 39,157 out · 64,233 cached (80%) · z-ai/glm-5.2
critique:    $0.0218 · 31,622 in / 22,504 out · 26,130 cached (83%) · z-ai/glm-5.2
security:    $0.0192 · 19,211 in / 5,980 out  · 15,342 cached (80%) · z-ai/glm-5.2
tests:       $0.0217 · 14,132 in / 7,550 out  · 11,041 cached (78%) · z-ai/glm-5.2
description: $0.0114 · 14,927 in / 3,123 out  · 11,720 cached (79%) · z-ai/glm-5.2

[tinysweeper]

Enforce a pinned fee_limit even when the response omits the field

The fee_limit comparison is gated on both sides being Some, so a request that pinned a fee limit is not protected against a response that omits the field entirely. if let (Some(expected), Some(actual)) only fires when the request set a limit and the response carries one; when fee_limit_sun is Some(expected) but optional_varint(...) returns None, the bindings don't match and the check is silently skipped, so verification passes despite the node having dropped the limit the caller asked for. The docstring frames this field as "the fee_limit the request specified," and the only test (a_raised_fee_limit_is_rejected_when_the_request_pinned_one) exercises the both-present-and-differ path, leaving the request-pinned/response-absent path uncovered. A pinned fee limit that the node refuses to honour is exactly the kind of alteration this function exists to catch.

[RULE] Fee limit pinned by the request is not enforced when the response omits it ·

[tinysweeper]

Cover the fee_limit branch where the request pins one but the transaction omits

The fee_limit consistency check only fires when both the requested fee_limit_sun and the on-wire field 18 are Some. When the caller pins a fee limit but the node omits field 18 entirely, the if let does not match and verification silently returns Ok(()). No test exercises this path — every TRC-20 test either sets both to the same value, sets both to different values, or sets both to None. A node that strips fee_limit to dodge the check is the exact adversary this function exists to catch, and a regression that changed this fall-through to an error (or vice versa) would break no existing test.

[RULE] new branches with no coverage ·

[tinysweeper]

What this change touches

6 files, +880 -10 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/tx<br/>3 files +693 -9<br/>6 findings"]:::blocking
  n1["src/tx/proto<br/>1 file +172 -0"]:::changed
  n2["root<br/>1 file +14 -0<br/>2 findings"]:::blocking
  n3["src<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/tx	changed	3	+693 -9	6 (high)
src/tx/proto	changed	1	+172 -0	—
(root)	changed	1	+14 -0	2 (high)
src	changed	1	+1 -1	—

Changed files

src/tx

• src/tx/mod.rs
• src/tx/proto.rs
• src/tx/tron.rs

src/tx/proto

• src/tx/proto/test.rs

(root)

• Cargo.toml

src

• src/lib.rs

[senamakel]

Second commit: tx-codec

Wiring the new verify_contract into tinyhumansai/openhuman surfaced the reason it could not be used there:

error[E0433]: cannot find `tx` in `tinywallet`
    |
133 |     let recomputed_txid = tinywallet::tx::tron::recompute_txid(...)
    |                                       ^^ could not find `tx` in `tinywallet`
    |
note: found an item that was configured out

OpenHuman takes this crate without tx, deliberately — that gate pulls bitcoin and its native secp256k1 build, and transaction building happens in the loaded wallet module instead. But verification is not building. It is &[u8] walking plus sha2, and a host that has moved signing elsewhere still has to check what a Tron node handed back before it signs.

So tx-codec splits along that line:

	Gate	Needs
tx::proto, tx::tron::{recompute_txid, verify_transfer, verify_contract, digest, attach_signature, signature_hex}	tx-codec	sha2
tx::tron::sign, tx::btc, tx::evm, tx::solana, tx::rlp	tx	bitcoin

bitcoin now gates exactly one function in tx::tron. Note digest, attach_signature and signature_hex are on the codec side: they are pure, and they are precisely what a host doing its own k256 signing over a returned digest needs.

tx = ["tx-codec", ...], so nothing that took tx before sees any change.

Measured, not asserted:

$ cargo tree -p tinywallet --no-default-features --features "tron,tx-codec" \
    -e normal --prefix none | sort -u | wc -l
22
$ ... | grep -cE "^(bitcoin|secp256k1) "
0

And on the consuming side, with openhuman enabling tx-codec:

$ cargo tree --features "$(bash scripts/ci/product-features.sh)" \
    -e normal --prefix none | sort -u | grep -icE "^bitcoin |^secp256k1 "
0

Gate matrix — all check clean: no features · tron · tron,tx-codec · the full host set. --all-features keeps 297 + 7 + 9 + 18 tests green, clippy and fmt clean.

[tinysweeper]

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.0788 · 69,777 in / 22,750 out · 45,095 cached (65%) · z-ai/glm-5.2
critique:    $0.0326 · 23,459 in / 11,213 out · 18,996 cached (81%) · z-ai/glm-5.2
security:    $0.0204 · 15,450 in / 4,111 out  · 1,792 cached (12%)  · z-ai/glm-5.2
tests:       $0.0142 · 15,038 in / 4,229 out  · 11,719 cached (78%) · z-ai/glm-5.2
description: $0.0118 · 15,830 in / 3,197 out  · 12,588 cached (80%) · z-ai/glm-5.2

[tinysweeper]

Enforce a pinned fee_limit even when the response omits the field

The fee_limit comparison only runs when both sides are Some. When the request pins fee_limit_sun: Some(expected) but the transaction omits field 18 entirely, proto::optional_varint returns None, the if let pattern does not match, and verification succeeds without checking anything. A node can strip the fee_limit from the response and the caller's stated limit is never enforced. The fix is to treat Some(expected) with None actual as a mismatch, not just Some vs Some with unequal values.

[RULE] guard ·

[tinysweeper]

Cover the fee_limit branch where the request pins one but the transaction omits

a_raised_fee_limit_is_rejected_when_the_request_pinned_one covers Some vs Some with different values, and a_well_formed_trc20_transfer_verifies_structurally covers Some vs Some with equal values. No test covers the case the first finding describes: the request pins fee_limit_sun: Some(…) and the transaction omits field 18, which currently passes silently. A failing test for that case should precede the fix.

[RULE] test ·

[tinysweeper]

Enforce a pinned fee_limit even when the response omits the field

The guard only fires when both the request pinned a fee_limit and the response includes field 18. If the node strips fee_limit from raw_data while the caller set fee_limit_sun = Some(_), the let fails to match and verification passes silently — exactly the class of discrepancy this function exists to catch. The caller's intent (a capped fee) is unenforceable, and a node can bypass the pin by omission rather than substitution.

[RULE] , ·

[tinysweeper]

Enforce a pinned fee_limit even when the response omits the field

Still standing

The fee_limit enforcement only fires when both the request and the transaction carry a value:

if let (Some(expected), Some(actual)) = (
    *fee_limit_sun,
    proto::optional_varint(&raw_fields, 18, "Transaction.raw.fee_limit")?,
)
&& actual != expected
{
    return Err(untrusted("the transaction has a different fee_limit"));
}

When fee_limit_sun is Some but the transaction omits field 18, actual is None, the if let pattern does not match, and no error is returned. A node that drops fee_limit entirely bypasses the pinned-value check. The prior review raised this and it has not been addressed.

[RULE] Maintain at least 80% coverage of meaningful library behavior and add/update tests with every behavior change. ·

[tinysweeper]

Cover the fee_limit branch where the request pins one but the transaction omits

Still standing

No test covers the case where fee_limit_sun is Some and the transaction omits field 18. The existing fee_limit test, a_raised_fee_limit_is_rejected_when_the_request_pinned_one, uses a transaction that includes a different fee_limit. There is no test that builds a TRC-20 transaction without field 18 and asserts the pinned value is still enforced. Because the if let guard requires both to be Some, such a transaction currently passes silently — and no test would catch that regression.

[RULE] Maintain at least 80% coverage of meaningful library behavior and add/update tests with every behavior change. ·

[tinysweeper]

Cover the TRC-20 contract_address mismatch branch

New finding

The TRC-20 branch checks that contract_address (field 2) matches the expected recipient:

if proto::one_bytes(&payload, 2, "TriggerSmartContract.contract_address")?
    != expected_recipient
{
    return Err(untrusted("the transaction targets a different contract"));
}

No test sends a TRC-20 transaction whose contract_address differs from the requested to. Every TRC-20 test — the well-formed case, the calldata mismatch, the call_value smuggling, and the fee_limit mismatch — builds the transaction with the correct contract address. If this check were removed, all existing tests would still pass. The native-recipient mismatch is tested by a_recipient_present_but_not_as_the_to_address_is_rejected, but the TRC-20 contract-address check is a distinct branch on a distinct field of a distinct contract type.

[RULE] Maintain at least 80% coverage of meaningful library behavior and add/update tests with every behavior change. ·