feat(vm): add ABI semantic validation for /wallet/deploycontract#6703
feat(vm): add ABI semantic validation for /wallet/deploycontract#6703yanghang8612 wants to merge 2 commits intotronprotocol:developfrom
Conversation
…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.
yanghang8612
changed the title
| if (raw == null || raw.isEmpty()) { | ||
| return "type must not be empty"; | ||
| } | ||
| String t = raw.trim(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Thanks @alan-eth — that lands. The whole point of this validator is that "passed validation" should match "what gets persisted"; accepting " uint256 " breaks the invariant silently and the proto write never normalizes it back. Adding the upfront whitespace reject is a cleaner contract than threading a trim() through the write path.
Going in the next commit.
| case "fallback": | ||
| return SmartContract.ABI.Entry.EntryType.Fallback; | ||
| case "error": | ||
| return SmartContract.ABI.Entry.EntryType.Error; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Good catch. Adding case "receive" to both TvmTestUtils.getEntryType and PublicMethod.getEntryType so the validator's Receive handling (payability + IO checks) is actually reachable from test helpers — otherwise any test that exercises a receive entry bounces off "unknown entry type" before it hits the rules the validator is there to enforce. Folds into the same commit as the whitespace fix above.
…elpers - AbiValidator now rejects types with leading/trailing whitespace upfront with a precise error, so passing validation implies the persisted type bytes equal the checked bytes. - PublicMethod / TvmTestUtils helpers map "receive" in getEntryType and exempt receive from the no-inputs guard, so test ABIs with a receive entry reach the validator's Receive branch instead of falling to UNRECOGNIZED.
| private AbiValidator() { | ||
| } | ||
|
|
||
| public static void validate(ABI abi) throws ContractValidateException { |
There was a problem hiding this comment.
Is validate(ABI abi) too strict? It may reject legitimate ABIs generated by older or non-standard compilers.
4 participants
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:
uint/intshorthand and out-of-rangeuintN/intNwidthsbytesNwith N outside [1, 32]fixed/ufixedfamilyconstructor/fallback/receivereceiveto be payable (legacypayableflag honored)fallback/receivecarrying inputs or outputsfunction; require a name foreventonly when notanonymoustuple/tuple[]/tuple[N]permissive, since the proto schema cannot representcomponentsAlso reuse the validator in PublicMethod.jsonStr2Abi and TvmTestUtils.jsonStr2Abi so test/util ABI parsing and request-handling share the same rule set.