Skip to content

fix(module): refuse an EVM transaction with no recipient - #117

Merged
M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/evm-transfer-requires-recipient
Oct 23, 2026
Merged

M3gA-Mind merged 2 commits into
tinyhumansai:mainfrom
M3gA-Mind:fix/evm-transfer-requires-recipient

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Contributor

Summary

The module rebuilds an EVM transaction from its wire fields before hashing and signing it. A blank or whitespace-only to was mapped to None, which in a legacy EVM transaction means contract creation: the wallet would sign a transaction that deploys its data as a new contract and sends value to it. No wallet flow (transfers, swaps, bridges, dapp calls) creates contracts, and the comment beside the code already said so. A blank recipient is now refused with InvalidInput("EVM recipient is required") before anything is signed. Both callers (signing payload construction and signature attachment) go through this one function.

Related issue

None.

API or behavior changes

An EVM TransactionSpec with an empty or whitespace-only to is now refused (InvalidInput) instead of being signed as a contract creation. No public API change.

Validation

  • cargo fmt --all -- --check (touched files, rustfmt edition 2024)
  • N/A: cargo clippy --all-targets --all-features -- -D warnings: left to CI
  • N/A: cargo build --all-targets --all-features: left to CI
  • N/A: cargo test --all-features: only the targeted test below ran locally; CI runs the suite

Tests

service::test::transaction_shape_and_key_guards_refuse_cross_chain_inputs asserted that a blank recipient produced a contract creation; it now asserts "" and " " are refused with InvalidInput.

cargo test -p tinywallet-module --lib -- transaction_shape_and_key_guards_refuse_cross_chain_inputs

  • Before the fix: FAILED. 0 passed; 1 failed (the blank recipient was accepted).
  • After the fix: ok. 1 passed; 0 failed.

Documentation

None needed; the inline comment is updated.

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

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: dbdbf147-546e-400a-a4f6-4e0437347270
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@tinysweeper

tinysweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 92bfc1c4b7ee22fea3e9029735ea67ce3b7f34b9

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 2 of 2 files.
Pull request purpose (title): fix(module): refuse…

Diagnostics and supporting details

Full summary

Pull request purpose (title): fix(module): refuse an EVM transaction with no recipient. The change replaces the silent mapping of a blank or whitespace-only EVM recipient to `to: None` (which in a legacy EVM transaction means contract creation) with an explicit `InvalidInput("EVM recipient is required")` refusal in the shared `evm_transaction` constructor, and updates the corresponding unit test to assert the refusal for `""` and `" "`. The latest push delta is unavailable, so only whole-pull-request evidence is described.

Review snapshot

Change surface Files Review signal Count
Production 1 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

An EVM `TransactionSpec` with a blank or whitespace-only `to` was previously encoded as `to: None`, meaning a legacy contract creation that deploys `data` and sends `value` to the new contract. The `evm_transaction` constructor now trim-checks `to` and returns `Failure::InvalidInput("EVM recipient is required")` before constructing the transaction; the struct literal unconditionally uses `Some(to.clone())`, reachable only for non-blank input. The description lane reports both callers (`build_unsigned` and `attach_signature`) route through this one function, so both paths refuse consistently. No public API change; specs previously accepted as contract creation now error.

Features

  • Modified — Refuse blank EVM recipients in `evm_transaction`: A blank or whitespace-only recipient now yields `InvalidInput` before hashing/signing instead of a contract-creation transaction. The security and description lanes report the guard sits before any use of `to`, both callers surface the new `Failure::InvalidInput`, the variant matches the existing `Failure` enum, and no security regression or rule violation appears in the changed files. The critique lane found no active findings and notes it could not see whether `sign`/`digest` already rejected an empty-string `Some(to)`, concluding nothing regresses either way. (crates/tinywallet-module/src/service/mod.rs fn evm_transaction(spec: &TransactionSpec) -> Result<tx::evm::LegacyTransaction,)
  • Modified — Update recipient guard test to cover empty and whitespace-only inputs: The test now iterates over `""` and `" "`, builds an EVM spec for each, and asserts `matches!(evm_transaction(&creation), Err(Failure::InvalidInput(_)))`, replacing the previous assertion that a blank recipient produced `to.is_none()`. (crates/tinywallet-module/src/service/mod_tests.rs fn transaction_shape_and_key_guards_refuse_cross_chain_inputs() {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The critique lane reports the change is sound: the refusal replaces the `None` mapping and both callers surface the new `Failure::InvalidInput`; it found no active findings.
  • Lane summary: The change is sound: `evm_transaction` now refuses a blank recipient instead of mapping it to `None` (contract creation), and both callers in `mod.rs` surface the new `Failure::InvalidInput`. The only visible consequence I cannot see is whether `tx::evm::LegacyTransaction::sign`/`digest` previously rejected an empty-string `Some(to)` anyway, which would mean no behaviour change at all — but either way nothing regresses, since the untouched test only asserted the old `to.is_none()` mapping for a blank recipient. The updated test covers both "" and " " and asserts the range `InvalidInput` correctly.

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The security lane reports the blank recipient is now refused via the existing `Failure` enum in `evm_transaction`, with no security regression, no widened untrusted-input path, and no rule violation in the two changed files.
  • Lane summary: The change tightens EVM construction: a blank recipient is now refused with `Failure::InvalidInput` in `evm_transaction` instead of being mapped to `None` (contract creation), and the unit test covers both an empty and whitespace-only recipient. The error path matches the existing `Failure` enum and the test asserts on the same variant, so this looks sound. I found no security regression, no untrusted-input path that the change widens, and no rule violation in the two changed files.

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The tests lane reports the behaviour change and its updated test land together, the error path is covered for both `""` and `" "`, and no reportable defect was found.
  • Lane summary: The change replaces an empty-recipient EVM transfer from being silently mapped to contract creation (`to: None`) with an explicit `InvalidInput` refusal, and the module test is updated to assert the new behaviour for both `""` and `" "`, plus keeps the error path covered via `matches!(..., Err(Failure::InvalidInput(_)))`. The regression the change introduces (a transfer that previously signed now errors) is pinned by a test that fails if the guard is removed, and the guard exists on a path reached from the public signing API. I found no reportable defect.

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The commits lane reports no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files.

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The description lane reports the diff matches the description: the guard is placed before hashing or signing, both callers route through `evm_transaction`, and the test asserts refusal for `""` and `" "`; no defects found.
  • Lane summary: The diff matches the description: a blank or whitespace-only EVM `to` is now refused with `InvalidInput("EVM recipient is required")` before hashing or signing, both callers (`build_unsigned` and `attach_signature`) route through `evm_transaction`, and the test that previously asserted contract creation now asserts refusal for `""` and `" "`. The description is accurate and sufficient; no defects found in the change itself.
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000333
  • Tokens: 114909 input · 2509 output · 16384 cached · 116 embedding
  • Updated: 2026-10-23T09:21:51Z
Head State Pass summary
7292a387bf2c ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T02:28:10Z)
926f2d41be48 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T08:20:50Z)
92bfc1c4b7ee ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T09:21:51Z)

tinysweeper 0.1.2

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

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0004

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 23, 2026
evm_transaction mapped a blank `to` to None, which signs a contract
creation. No wallet flow creates contracts, so a blank recipient is now
InvalidInput instead of a transaction that deploys its data as code.
@M3gA-Mind
M3gA-Mind force-pushed the fix/evm-transfer-requires-recipient branch from 7292a38 to 926f2d4 Compare October 23, 2026 08:20
@M3gA-Mind
M3gA-Mind merged commit ba19415 into tinyhumansai:main Oct 23, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant