Skip to content

feat(vm): add ABI semantic validation for /wallet/deploycontract#6703

Open
yanghang8612 wants to merge 1 commit intotronprotocol:developfrom
yanghang8612:feat/issue-6674-abi-validation
Open

feat(vm): add ABI semantic validation for /wallet/deploycontract#6703
yanghang8612 wants to merge 1 commit intotronprotocol:developfrom
yanghang8612:feat/issue-6674-abi-validation

Conversation

@yanghang8612
Copy link
Copy Markdown
Collaborator

Add a dedicated AbiValidator and invoke it inside the existing CreateSmartContract branch of Wallet.createTransactionCapsule, so both the HTTP /wallet/deploycontract path and the gRPC deployContract path fail fast on malformed ABI input with a field-path-anchored error.

Rules align with go-ethereum's accounts/abi parser:

  • reject uint/int shorthand and out-of-range uintN/intN widths
  • reject bytesN with N outside [1, 32]
  • reject the entire fixed/ufixed family
  • reject malformed array suffixes
  • reject duplicate constructor/fallback/receive
  • require receive to be payable (legacy payable flag honored)
  • reject fallback/receive carrying inputs or outputs
  • reject UnknownEntryType
  • require a name for function; require a name for event only when not anonymous
  • keep tuple / tuple[] / tuple[N] permissive, since the proto schema cannot represent components

Also reuse the validator in PublicMethod.jsonStr2Abi and TvmTestUtils.jsonStr2Abi so test/util ABI parsing and request-handling share the same rule set.

…nprotocol#6674)

Add a dedicated AbiValidator and invoke it inside the existing
CreateSmartContract branch of Wallet.createTransactionCapsule, so both
the HTTP /wallet/deploycontract path and the gRPC deployContract path
fail fast on malformed ABI input with a field-path-anchored error.

Rules align with go-ethereum's accounts/abi parser:

- reject `uint`/`int` shorthand and out-of-range `uintN`/`intN` widths
- reject `bytesN` with N outside [1, 32]
- reject the entire `fixed`/`ufixed` family
- reject malformed array suffixes
- reject duplicate `constructor`/`fallback`/`receive`
- require `receive` to be payable (legacy `payable` flag honored)
- reject `fallback`/`receive` carrying inputs or outputs
- reject UnknownEntryType
- require a name for `function`; require a name for `event` only when
  not `anonymous`
- keep `tuple` / `tuple[]` / `tuple[N]` permissive, since the proto
  schema cannot represent `components`

Also reuse the validator in PublicMethod.jsonStr2Abi and
TvmTestUtils.jsonStr2Abi so test/util ABI parsing and request-handling
share the same rule set.
@github-actions github-actions Bot requested a review from CodeNinjaEvan April 24, 2026 01:36
@yanghang8612 yanghang8612 changed the title feat(vm): add ABI semantic validation for /wallet/deploycontract (#6674) feat(vm): add ABI semantic validation for /wallet/deploycontract Apr 24, 2026
if (raw == null || raw.isEmpty()) {
return "type must not be empty";
}
String t = raw.trim();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice helper layout — checkType returning either null or a short reason makes the call site read very cleanly, and isolating the regex/base-type tables at the top is great for future tweaking. ✨

One subtle gap: t = raw.trim() lets a type like " uint256 " pass validation, but the validator never normalizes the proto — the on-chain Param.type keeps the original whitespace. Per the linked issue's "fewer inconsistencies in stored ABI metadata" goal, would it make sense to reject whitespace-padded types upfront, e.g.:

if (!raw.equals(raw.trim())) {
  return "type must not contain leading/trailing whitespace";
}

That way the validator's "this passed" implies "what gets persisted is exactly what got checked", which matches the stated goal of strict-matching tooling compatibility.

case "fallback":
return SmartContract.ABI.Entry.EntryType.Fallback;
case "error":
return SmartContract.ABI.Entry.EntryType.Error;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great call adding case "error" here — without it any test ABI carrying an error entry would fall into UNRECOGNIZED and now hit the new "unknown entry type" rejection. Nice symmetry with PublicMethod.getEntryType which already had it. 🎯

Minor follow-up: both this helper and PublicMethod.getEntryType still omit case "receive", even though the new validator has dedicated handling for Receive (payability + IO checks). Any test that wants to exercise a receive entry through these helpers will still get UNRECOGNIZED and bounce off the new rejection. Worth adding the one-line case in both files while you're here, so the validator's full surface is reachable from test utilities?

@halibobo1205 halibobo1205 added this to the GreatVoyage-v4.8.2 milestone Apr 24, 2026
@halibobo1205 halibobo1205 added topic:vm VM, smart contract topic:api rpc/http related issue labels Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:api rpc/http related issue topic:vm VM, smart contract

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Add ABI semantic validation for /wallet/deploycontract

3 participants