Repository navigation
feat(api): bound transaction field occurrences before parsing - #10
Open
waynercheung wants to merge 1 commit into
Open
waynercheung wants to merge 1 commit into
waynercheung wants to merge 1 commit into
Conversation
waynercheung
force-pushed
the
feat/tx-admission-guard
branch
from
October 10, 2026 08:18
8daf9fc to
9481e6c
Compare
protobuf-java materializes objects for repeated sub-message and unknown-field occurrences. A submitted Transaction containing many empty ret entries (2 bytes on the wire each) can therefore expand to tens of times its wire size on the heap during parseFrom. Existing transaction-level checks run after this allocation. The contract payload (Any.value) is also unpacked by TransactionCapsule.getOwner before signature verification, exposing its nested messages to the same amplification. Scan the wire bytes before parsing on the API entry points that accept a binary Transaction, and reject the request once it exceeds 50,000 field occurrences: - TransactionAdmissionGuard counts every tag with CodedInputStream, recurses into known sub-messages by descriptor, and recurses into the effective contract payload with the type getOwner would unpack, following protobuf merge semantics. Group encoding and nesting deeper than 32 are rejected. Block, database and P2P parsing do not use it. - For gRPC, every method whose request is a Transaction gets a request marshaller that reads the request with the inbound size bound, runs the guard and hands the same bytes to the original marshaller. A rejection returns a sentinel instead of throwing, and a handler wrapper closes the call with INVALID_ARGUMENT before the service method runs. This affects BroadcastTransaction, GetTransactionSignWeight, GetTransactionApprovedList, GetShieldTransactionHash and CreateCommonTransaction. - /wallet/broadcasthex runs the guard before Transaction.parseFrom. - Admission rejections use one fixed client message and a sampled DEBUG log, at most one line per 10 seconds per transport, without stack traces. In a one-week mainnet sample, the maximum observed count was 922 field occurrences. This is an API admission policy, not a consensus-validity limit: previously tolerated encodings and unusually complex transactions may now be rejected at these entry points. Consensus, execution and block validation are unchanged.
waynercheung
force-pushed
the
feat/tx-admission-guard
branch
from
October 10, 2026 08:28
9481e6c to
6b4019f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds an admission check that counts field occurrences in a submitted
Transactionbefore it is parsed, and rejects it once it exceeds 50,000. It applies to the API entry points that accept a binaryTransaction:BroadcastTransaction,GetTransactionSignWeight,GetTransactionApprovedList,GetShieldTransactionHash,CreateCommonTransaction(theWalletmethods whose request type isTransaction)/wallet/broadcasthexJSON transaction endpoints and gRPC methods with other request types are unchanged.
Changes:
TransactionAdmissionGuardscans the wire bytes withCodedInputStream(the decoder the generated parser uses) and counts every tag, known or unknown. It recurses into sub-messages by descriptor;bytesandstringfields are leaves, so large bytecode counts once. For eachContractit first resolves the effective type,type_urlandvalueunder protobuf merge rules (last occurrence wins, repeatedparameteroccurrences merge), then recurses into thatvaluewith the payload typeTransactionCapsule.getOwnerwould unpack. Avaluethat is never unpacked (overwritten, mismatchedtype_url, unknown type) stays a leaf. Group encoding and nesting deeper than 32 are rejected. The guard checks structure only; it does not validate field contents such as UTF-8 in strings.GuardedTransactionMarshallerreplaces the request marshaller of those gRPC methods. It reads the request with the same bound asmaxInboundMessageSize, runs the guard and passes the same bytes to the original marshaller, so parsing is unchanged for accepted requests.UNKNOWNand logs a SEVERE stack trace each time.AdmissionCheckedHandlerrecognizes the sentinel by identity and closes the call withINVALID_ARGUMENT; the service method is never invoked.RpcService.guardTransactionMethodsrebuilds each service definition with the guarded marshaller and handler for methods whose proto input type isTransaction. All services inRpcApiService,RpcApiServiceOnSolidityandRpcApiServiceOnPBFTare registered through it, so aTransactionmethod added later is covered too. Only the five FullNodeWalletmethods match today.BroadcastHexServletruns the guard beforeTransaction.parseFrom.transaction rejected by admission check, and a sampled DEBUG log (AdmissionRejectLog: at most one line per 10 seconds per transport, with the count since the last line, no stack trace).Block, database and P2P parsing do not use the guard: a valid block must never fail a local, non-consensus limit.
Why are these changes required?
protobuf-java materializes objects for repeated sub-message and unknown-field occurrences. A submitted
Transactioncontaining many emptyretentries (2 bytes on the wire each) can therefore expand to tens of times its wire size on the heap duringparseFrom(a 4 MiB request retained about 265 MiB in a local measurement). Emptyraw.contractandraw.authsentries and unknown fields with distinct numbers behave the same way. Existing transaction-level checks, such as the transaction size check and signature verification, run after this allocation.The outer parse keeps the contract payload (
Any.value) as bytes, butgetOwnerunpacks it before signature verification, exposing its nested messages to the same amplification. Checking only the outer message is not enough.Counting occurrences limits the structural complexity of the scanned schema, and with it the related object allocation, without field-specific rules. It is not an exact object count or heap size; the existing byte limits on requests stay in place.
Behavior changes (release note)
Transport and admission behavior was compared on the same inputs using a local differential harness with stubbed business handlers (gRPC) and a copy of the base servlet with a mocked
Wallet(HTTP). Production business-response mappings were additionally checked against source.INVALID_ARGUMENTwith descriptiontransaction rejected by admission check. Previously, inputs that failed to decode returnedUNKNOWN("Application error processing RPC") with a SEVERE log per request; inputs that decoded reached the service method and returned its result (OKwith an error code in the body for most methods, mostlyINTERNALforGetShieldTransactionHash)./wallet/broadcasthex:{"Error":"transaction rejected by admission check"}with HTTP 200. Previously a decodable request returnedresult/code/txid, and a malformed one returnedinternal server error.txid.HTTP request failed, broadcasthex) are replaced by the sampled DEBUG line for these rejections. Exceptions raised inside service methods are unchanged.tron:grpc_service_latency_secondshas only anendpointlabel. Rejected calls are now closed through the intercepted call and are counted in this histogram; previously decode failures were closed by grpc directly and were not.Transaction(for exampleAny.type_url) still returninternal server error.maxInboundMessageSizestill returnsUNKNOWN. A corrupt gzip header still fails inside grpc before the marshaller runs.RESOURCE_EXHAUSTED.type_urldoes not itself cause an admission rejection; normal business validation is unchanged.Budget
This PR has been tested by:
New tests (61):
TransactionAdmissionGuardTest(28): counts and exact budget boundaries for each amplification shape; merge semantics (last type,type_urlandvaluewin, explicit empty overrides); payload recursion including shielded and multiple contracts; first-pass budget; malformed input; depth; concurrency; a corpus of valid transactions; the pinned budget value; no packed repeated scalar reachable fromTransactionor any contract type.AdmissionSupportTest(12): reason mapping, sentinel identity, bounded read edge cases, log sampling.RpcServiceAdmissionTest(12): real loopback Netty server. Valid requests pass (plain and gzip); over-budget, malformed, invalid UTF-8 and corrupt gzip bodies that reach the marshaller getINVALID_ARGUMENTwithout reaching the service method and without a grpc SEVERE log; interceptor permits are released; cancellation;disabledApistill applies; message size limits unchanged.RpcServiceRegistrationAdmissionTest(4): the definitions actually registered by eachRpcServicesubclass; exactly five methods guarded on FullNode, none elsewhere; each guarded marshaller uses the production budget andmaxMessageSize.BroadcastHexServletAdmissionTest(5): fixed response without callingWallet, one sampled DEBUG line without a stack trace.Selected existing suites, run locally on JDK 17 (aarch64) and JDK 8 (x86_64), 284 tests each, all passing:
In addition, all of
org.tron.core.services.*(125 classes) was run together with the new tests on JDK 17 and passed.checkstyleMainandcheckstyleTestpass.Follow up
RESOURCE_EXHAUSTED.KnownLengthstream to restore grpc's array fast path, and simplify the bounded read; to be measured separately.Extra details
Design choices that reviewers may ask about:
TransactionAdmissionGuard.countTransactionis public so that sampling tools use exactly the production counting logic.TOO_LARGEis a fallback. grpc enforces the same inbound limit first, but the bound keeps the marshaller's allocation independent of transport settings.Transaction.parser()) so that its configuration and behavior are kept.isDebugEnabled()check in the handler avoids evaluating the method name, the remote address and the argument array when DEBUG is off.