Skip to content

test: cover EOA self-calls with calldata - #342

Open
orkunsahin wants to merge 1 commit into
circlefin:mainfrom
orkunsahin:test/191-eoa-self-call-calldata
Open

test: cover EOA self-calls with calldata#342
orkunsahin wants to merge 1 commit into
circlefin:mainfrom
orkunsahin:test/191-eoa-self-call-calldata

Conversation

@orkunsahin

Copy link
Copy Markdown

Summary

  • Add regression coverage for an externally submitted EOA self-call with non-empty calldata.
  • Verify that the Arc transaction pool validator accepts from == to, zero value, and non-empty calldata.

References #191.

Context

The behavior originally reported in #191 no longer reproduces on the current Arc Testnet.

I verified a raw EOA self-call with 0x48656c6c6f ("Hello") successfully:

  • from == to
  • value = 0
  • non-empty calldata
  • transaction status: success

Test transaction:

0x1fc4b15af55eae09d93a2ef2b5c45a92ade3c3abf94561768617f2dd0bc40881

This change does not modify transaction validation behavior. It adds regression coverage to ensure the txpool validation path continues accepting this transaction pattern.

Testing

  • cargo fmt --check
  • cargo test -p arc-execution-txpool — 43 passed
  • cargo clippy -p arc-execution-txpool --all-targets --all-features -- -D warnings

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

Disclosure: I'm not affiliated with Circle — an external community contributor, not a maintainer. I have no write access to this repository, so any review state I set carries no merge authority and is advisory only. Please defer to Circle maintainers for the binding review. Also declaring an interest: I filed #191, so I'm reviewing a PR aimed at my own issue.


Thanks for doing the empirical work — your on-chain claim checks out in full, and I verified it independently rather than taking the hash on trust. But I don't think the test does what the PR says it does, and I'd rather say so than let #191 get marked covered by something that can't fail.

The transaction is exactly as described

0x1fc4b15a…c40881 on rpc.testnet.arc.network:

from        0x6f47178288a14d32ab0b0264c51875eb1d1a9ca4
to          0x6f47178288a14d32ab0b0264c51875eb1d1a9ca4   (from == to)
value       0x0
input       0x48656c6c6f                                  ("Hello")
type        0x2
status      0x1  success
gasUsed     21200   block 60309524   2026-09-03T21:57:57Z

And the pattern still estimates cleanly today — eth_estimateGas for that exact shape returns 21366. So the headline conclusion is right: the behavior in #191 does not reproduce on current Arc Testnet.

The test can't regress, because the rule it names has never existed in this repo

This is the part I'd push back on. The test lands in crates/execution-txpool/src/validator.rs, but nothing in that validator — or anywhere in the tree — implements the #191 restriction.

  • The error string from #191, External transactions to internal accounts cannot include data, appears in zero files at every published tagv0.6.0, v0.7.1, v0.7.2, v0.7.3, v0.8.0, and main. I checked with git grep per ref across all file types, not just .rs.
  • There is no from == to / self-call rule anywhere in crates/.
  • The Arc-specific rules in validate_one_with_state are exactly three: MAX_AUTHORIZATIONS_PER_TX, the blocklist check, and the denylist check. None of them inspects calldata in relation to the recipient.

So the new test constructs a well-formed legacy transaction, sends it through a validator with an empty denylist, and asserts it is Valid. That assertion holds for reasons entirely unrelated to self-calls — it would have passed unchanged on the day I filed #191, while the rejection was actively reproducing on testnet. A regression test that could not have caught the regression, and cannot fail if it returns, isn't coverage; and merging it under "References #191" creates a false signal that the path is now guarded.

The reason is structural, not a matter of picking a different assertion: the rejection was never enforced in this repo's published history, so the txpool crate has no rule to protect. Whatever produced that error lives outside what's published here.

What I'd suggest instead

  1. Close #191 on the evidence rather than testing around it. @thlucy already demonstrated a working self-call on 2026-06-28 (0xfa31e2ae…b88ae2) and I confirmed it that day; your transaction is a second, independent confirmation three months later. That is enough. Since I opened it, I'm happy to close it myself with your hash and thlucy's cited — just say the word, and I'd rather do that than leave a stale bug open against Circle.
  2. If maintainers do want durable coverage, it belongs at the layer that actually rejected the transaction — an execution-e2e test submitting an EOA self-call with calldata through the real submission path, alongside the existing suites there. That would fail if the restriction ever came back. A unit test against a validator with no such rule would not.

Minor, if the test stays

  • Every other test in this module uses the test_ prefix (test_validate_one_with_state, etc.); eoa_self_call_with_calldata_is_valid breaks that convention.
  • The match &mut tx { MockTransaction::Legacy { to, .. } => …, _ => unreachable!() } block has a dead arm by construction — tx is built by MockTransaction::legacy() two lines above, so the fallback can never be reached. Worth noting that no existing test in this file sets to at all; they rely on the mock's default, which is why there's no established helper to follow here.
  • with_gas_limit(30_000) is comfortably above the 21,080 intrinsic cost for five non-zero calldata bytes, so that's fine.

Leaving this as a comment rather than a change request — the code is harmless and your verification was sound, so whether to keep a passing-but-inert test is a maintainer judgement, not something I should block on. Usual caveat: no cargo or rustc in my environment, so the Rust side is source review plus git grep across published tags; the transaction and gas figures above are live RPC calls.

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.

2 participants