feat(jsonrpc): implement base eth_simulateV1 JSON-RPC method#6785
feat(jsonrpc): implement base eth_simulateV1 JSON-RPC method#6785APshenkin wants to merge 4 commits into
Conversation
APshenkin
changed the title
APshenkin
changed the title
bladehan1
dismissed
a stale review
Re-submitting as COMMENT (AI never APPROVE/REQUEST_CHANGES; reviewer judgment left to humans).
| TransactionContext ctx; | ||
| try { | ||
| ctx = executeOneConstantInternal(trxCap, headBlockCapsule, perCallChild, tracer); | ||
| } catch (RuntimeException e) { |
There was a problem hiding this comment.
[SHOULD] RuntimeException rethrow aborts the entire batch — diverges from per-call isolation
If any single call in simulateConstantContracts throws a RuntimeException (NPE in VM, downstream lookup failure, etc.), the catch rethrows and TronJsonRpcImpl.ethSimulateV1 translates that to a top-level JsonRpcInternalException — the client loses results for ALL 32 calls in the batch, including the ones that already succeeded with perCallChild.commit(). ETH eth_simulateV1 semantics expect per-call isolation: failed call returns status=0x0 + errorMessage, subsequent calls keep running.
Suggestion: Catch the RuntimeException, build a synthetic failed ProgramResult with setException(e), add it to outcomes, and continue to the next call. If fail-fast is intentional (e.g. sharedRoot may be corrupted after such a fault), say so in javadoc and document the deviation from ETH spec.
| private final Deque<List<Entry>> stack = new ArrayDeque<>(); | ||
| private long seq; | ||
|
|
||
| public BufferingSimulationTracer() { |
There was a problem hiding this comment.
[SHOULD] Per-call entry buffer is unbounded — quantify or cap
A single call can produce thousands of LOG opcodes (only energy-bounded) plus transfer entries; each goes into the root frame's ArrayList<Entry> with no upper bound. With MAX_SIMULATE_CALLS_PER_BLOCK=32, a malicious or pathological request can buffer hundreds of MB worth of Entry objects (per-Entry object header + topics + 32-byte data + byte arrays). The 32 cap was justified in the PR description as ~10 KB/call typical, but worst case is much higher.
Suggestion: Either add a stress test that pins the realistic upper bound (and document it), or introduce a per-call entry-count cap (e.g. 4096 or 100_000) that aborts the call with a clear errorMessage when exceeded.
|
|
||
| public static final class SimulateCallOutcome { | ||
|
|
||
| private final ProgramResult result; |
There was a problem hiding this comment.
[SHOULD] Move SimulateCallOutcome / SimulateOutcome out of Wallet.java into jsonrpc/types/
Wallet.java is already 4500+ lines and is a load-bearing god class. The new public static final class SimulateCallOutcome and SimulateOutcome are pure data carriers shaped exactly for the jsonrpc layer (they wrap ProgramResult + BufferingSimulationTracer.Entry) and have no semantic dependency on Wallet's instance state. Embedding them here further inflates the class and forces every consumer (TronJsonRpcImpl, future actuators, tests) to import Wallet.SimulateOutcome — a layering smell.
Suggestion: Promote both to top-level classes under framework/src/main/java/org/tron/core/services/jsonrpc/types/ (e.g. SimulateOutcome.java, SimulateCallOutcome.java); simulateConstantContracts returns the new top-level type. Wallet shrinks; jsonrpc DTO layer grows in the natural direction.
| .setOriginAddress(ByteString.copyFrom(owner)) | ||
| .setBytecode(ByteString.copyFrom(data)) | ||
| .setCallValue(value) | ||
| .setConsumeUserResourcePercent(100) |
There was a problem hiding this comment.
[SHOULD] Deduplicate CREATE-path magic numbers — extract a shared buildEvmCreateSmartContract helper
In buildSimulateTransactionCapsule, the CREATE branch hand-builds a CreateSmartContract with setConsumeUserResourcePercent(100) and setOriginEnergyLimit(1) — identical to Wallet.triggerConstantContract (around line 3084-3098 in HEAD) which does the same construction for triggerConstantContract deploy. These two magic constants encode Tron's EVM CREATE convention (also asserted at Program.java:851/867); duplicating them at three sites means a future tweak (say, supporting non-zero origin energy under a fork rule) must touch every copy.
Note: the TriggerSmartContract branch of buildSimulateTransactionCapsule already correctly reuses triggerCallContract + wallet.createTransactionCapsule; the asymmetry between branches is itself a smell.
Suggestion: Extract a Wallet#buildEvmCreateSmartContract(byte[] ownerAddress, byte[] code, long value) (or static on a SmartContractBuilders util) that sets the 100/1 convention in one place; have both buildSimulateTransactionCapsule and triggerConstantContract call it.
| return wallet.createTransactionCapsule(trigger, ContractType.TriggerSmartContract); | ||
| } | ||
|
|
||
| private SimulateBlockResult buildSimulateBlockResult(Wallet.SimulateOutcome outcome, |
There was a problem hiding this comment.
[SHOULD] Lift the 9 "no value" constants in BlockResult to inline field defaults
Anchored here on buildSimulateBlockResult because this PR-added builder constructs a SimulateBlockResult (extending BlockResult) and inherits the same "9 constant field assignments per construction" pattern.
In BlockResult.java lines 98-115, every construction sets nonce, sha3Uncles, logsBloom, receiptsRoot, difficulty, totalDifficulty, extraData, uncles, and baseFeePerGas to the same constant "no value" string — while mixHash at line 85 already follows the inline-default pattern (private String mixHash = ByteArray.toJsonHex(new byte[32]);).
Suggestion: In BlockResult.java, promote the 9 assignments to inline field initializers (matching the existing mixHash pattern at line 85), then delete the now-redundant assignments in the BlockResult(Block, boolean, Wallet) ctor. Both the existing call site and the new simulate-block construction get the defaults for free, and future schema drift becomes one-line.
| } | ||
|
|
||
| /** Reset per-call state. Drops any leftover frames and starts a fresh root. */ | ||
| public void beginCall() { |
There was a problem hiding this comment.
[SHOULD] beginCall and dropCall have identical bodies; constructor calls beginCall then caller calls it again
Both beginCall and dropCall are stack.clear(); stack.push(new ArrayList<>()); — same code, different intent. Two methods with the same body invite drift: if dropCall should ever do extra accounting (e.g. release pooled buffers, record metrics) the next contributor will only patch one. Separately, the constructor calls beginCall() and then Wallet.simulateConstantContracts calls tracer.beginCall() again on iter 0; both invocations are idempotent but the second is a no-op that masks the lifecycle.
Suggestion: Keep both method names for semantic clarity but have one delegate to the other (e.g. dropCall() { beginCall(); }). Drop the constructor's implicit beginCall() and let the caller drive lifecycle.
| System.currentTimeMillis()); | ||
| } | ||
|
|
||
| private static byte[] leftPad32(byte[] src) { |
There was a problem hiding this comment.
[SHOULD] leftPad32 + longToBytes reinvent DataWord
leftPad32(longToBytes(value)) is exactly new DataWord(value).getData(); the existing DataWord already does both steps and is used elsewhere in this file. Two private helpers that overlap with project infrastructure add maintenance surface for no win — and worse, the next person reading this won't know whether the duplicate exists for a subtle reason (endianness, padding convention) or just because the author didn't notice DataWord.
Suggestion: Replace leftPad32(longToBytes(amount)) and leftPad32(longToBytes(tokenId)) with new DataWord(value).getData(); delete leftPad32 and longToBytes.
| @JsonInclude(JsonInclude.Include.NON_NULL) | ||
| @Getter | ||
| @Setter | ||
| private String errorCode; |
There was a problem hiding this comment.
[NIT] errorCode field is declared but never assigned
private String errorCode; is declared with @JsonInclude(NON_NULL) and exposed via getter/setter, but nothing in the codebase calls the setter — the field is always omitted in the JSON output. Either dead code, or a forgotten plumbing step.
Suggestion: Either populate it (map EVM revert / exception types to a stable error-code enum) and add a test pinning the mapping, or delete the field until there's a concrete need.
| long headNum = head.getNum(); | ||
| byte[] headHash = head.getBlockId().getBytes(); | ||
| byte[] simBlockHash = Hash.sha3( | ||
| (SIMULATE_BLOCK_HASH_PREFIX + ByteArray.toHexString(headHash) + ":1").getBytes()); |
There was a problem hiding this comment.
[NIT] String.getBytes() uses platform default charset
Lines 1013 and 1046 use (SIMULATE_BLOCK_HASH_PREFIX + ByteArray.toHexString(headHash) + ":1").getBytes() with no charset argument. The content is ASCII so it works today, but the same class at line 908 already uses StandardCharsets.UTF_8 for the TRC10 topic — be consistent, and avoid any platform-default surprise that could perturb the synthetic-hash determinism contract.
Suggestion: Pass StandardCharsets.UTF_8 to both .getBytes(...) calls.
| private long callPenaltyEnergy; | ||
| @Getter | ||
| @Setter | ||
| private SimulationTracer simulationTracer; |
There was a problem hiding this comment.
[NIT] Documentation & test backlog — 5 rolled-up items
Grouped as one comment since all are doc-or-test asks (no production-code change). Anchor placed on Program.simulationTracer because the contract this field encodes underpins items 3 and 4.
-
simulationTracerconsensus-path javadoc —Program.java:152(this line). The field's invariant "MUST be null on consensus execution" is not enforced anywhere; setter is public. Add a javadoc spelling out the read-only-simulation-only contract; consider tightening setter visibility or addingArgs.getInstance().isSupportConstant()guard. -
transferAllTokenWithTracebyte-equivalence comment + test —Program.javaaround line 541. Theif (simulationTracer == null) { MUtil.transferAllToken(...); return; }early-return is the consensus path; any future hoisting (logger, Args read, getAccount call) above the null check silently breaks sync-from-genesis. Add an invariant comment and a SELFDESTRUCT-with-multi-TRC10 equivalence test (pre/post state byte-equal to the pre-refactor path). -
DELEGATECALL / CALLCODE skip integration test —
Program.javaaround lines 1169 / 1188 / 1803 / 1822 (4 hook sites). Themsg.getOpCode() != Op.DELEGATECALL && msg.getOpCode() != Op.CALLCODEguard prevents synthetic Transfer logs but has no test coverage; if a future cleanup drops any single guard, clients see a ghost transfer log and rebuild incorrect ledgers with no CI signal. Add an integration test: Adelegatecalls B and B doesaddress.transfer(value); asserttraceTransfers=truereturns NO ERC-7528 Transfer entry. -
TRC10Transfer topic[0] lock-down (javadoc + test) —
TronJsonRpcImpl.javaaround line 1071.TRC10_TRANSFER_TOPIC_HEXis derived from the literal signature string and is a Tron-private extension; clients will hard-code this hex. Add a javadoc "Tron private extension; signature is a stable client contract" and a unit test asserting the exact hex value of the topic so any future signature edit fails CI. -
validationETH-spec divergence javadoc —SimulateV1Args.javaaround line 29. ETH specvalidation=trueenables signature + nonce + gas-pricing checks; here it's only "sender activated + balance >= callValue". Clients porting code from geth will hit silently different behavior. Add a javadoc listing exactly which checks run and noting the divergence.
2 participants
What does this PR do?
Implements geth's
eth_simulateV1on java-tron's JSON-RPC surface for the MVP trading-flow use case: a single round-trip that runs N dependent calls against current head state and returns each call's effect plus synthetic transfer logs.The endpoint is opt-in via existing
eth_call-style flags; no existing behaviour changes.Why are these changes required?
This is first step to resolve #6199
Tron's JSON-RPC exposes
eth_callbut noteth_simulateV1. Two concrete consumers benefit:eth_simulateV1lets the wallet run the unsigned transaction against current head state and decode the resulting transfer logs directly into a human-readable diff.Both consumers simulate against current head state only — they don't need historical-block context or state overrides. The current implementation covers those cases end-to-end and is sufficient for what we expect to be the majority of
eth_simulateV1usage on Tron.Future work needed for the remaining use cases (debugging historical txs, what-if analysis with state overrides) requires changes in the archive node to support simulation on a specific block +
stateOverrides/blockOverrides. That's intentionally out of scope here.JSON-RPC surface
blockStateCalls: [{ calls: [...] }]. Multi-block /blockOverrides/stateOverrides→-32602.blockOverridesandstateOverridesare excluded by design — both consumers in the Motivation section simulate against current head state only, and supporting overrides would require the same archive-node plumbing called out as future work (rewinding to a specific block, applying account/storage patches before VM execution). Rejecting them with a clear error is better than silently ignoring them. Hard cap of 32 calls per block — geth's defaults (5000/block, 10000 total) are tuned for general-purpose use; our concrete cases (trading-flow approval+swap+settle, wallet preview of a single user-signed tx that fans out a few internal calls) realistically stay under ~10. Capping at 32 leaves comfortable headroom while bounding per-request memory: at ~10KB of accumulated state per call, the shared root's in-memory cache stays well under ~1MB worst-case (vs ~50MB at 5000). Anything beyond that should either be a separate request or signal misuse.blockNumOrTag: only"latest"and"pending"accepted; both resolve to the head block (Tron has instant finality — no mempool state distinct from latest).traceTransfers: truesynthesizes logs at the ERC-7528 native pseudo-address (0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE), distinguished bytopic[0]:Transfer(address,address,uint256)for native TRX moves.TRC10Transfer(address,address,uint256,uint256)for TRC-10 moves —topic[3]carriestokenIdso consumers can filter logs by asset via topics;datacarriesamount.returnFullTransactions: truereturnsblock.transactionsasTransactionResultobjects (synthetic deterministichash,gasPrice = "0x0",nonce = "0x0",v/r/szero,blockHash = keccak256("sim:" + headHash + ":1")). Defaultfalsereturns hash strings.validation: true— Tron-flavored. Geth's checks (baseFee/gasPrice/nonce) don't apply to Tron, so we pre-check per call: sender account exists and sender balance ≥callValue. Failed pre-check →status: "0x0"witherrorMessage, VM not invoked. Defaultfalsepreserves the existing constant-call permissive behaviour.Tracing
Five hook sites per transfer kind, each firing after the real balance change succeeds:
VMActuator.call()(depth 0)MUtil.transferMUtil.transferTokenVMActuator.create()(depth 0)MUtil.transferMUtil.transferTokenProgram.callToAddress(depth ≥1)addBalancepairaddTokenBalancepairProgram.callToPrecompiledAddress(depth ≥1)addTokenBalancepairProgram.suicide/suicide2MUtil.transfertransferAllTokenWithTracehelper — snapshotassetMapV2beforeMUtil.transferAllToken, emit one entry per non-zero assetDELEGATECALL/CALLCODEare explicitly skipped (no real value transfer even whensenderAddress != contextAddress).A per-frame buffer (
BufferingSimulationTracer) with a unifiedseqcounter interleaves explicitLOGopcodes with both synthetic kinds in emission order. Reverted frames drop their entries;logIndexstill increments through gaps (matches geth'slogtracer.go:128).Implementation
SimulationTracer(enterFrame/exitFrame/revertFrame/onTransfer/onTokenTransfer/onLog). Default implBufferingSimulationTracerowns the frame stack andseqcounter.VMActuatorgets opt-in setters (setInjectedRootRepository,setSimulationTracer). When the injected root is null, the existing fresh-root code path runs unchanged.Programpropagates the tracer into childPrograminstances at every sub-call origination site so nested CALL/CREATE moves are captured.Wallet.simulateConstantContractsis the new entry point. It builds the shared root + per-call child Repositories and shares the per-call execute body with the existingcallConstantContractvia a new privateexecuteOneConstantInternalhelper.SimulateV1Args,SimulateBlock(uses@JsonAnySetterto detect forward-incompatible field names),SimulateCallResult,SimulateBlockResult extends BlockResult. ReusesLogFilterElement,CallArguments,TransactionResult(additive raw-fields constructor for synthetic full-tx output).This PR has been tested by:
Unit
20 tests total:
Unit (
EthSimulateV1ArgsTest, 11 tests)-32602input-validation surface, JSON round-trip ofSimulateBlockResult, non-numerictokenIdrejection. MockedWallet, no chain context.Integration (
EthSimulateV1IntegrationTest, 9 tests,BaseTest+ LevelDB)stateSharingAcrossCalls—set(42)→get()returns 42 in one simulate; on-chain slot unchanged.revertIsolatesPerCall—set(99)→setRevert(123)→get()returns 99.validationRejectsUnactivatedSender—validation: truewith a never-seenfrom→"sender account does not exist".validationRejectsInsufficientBalance—validation: truewithvalue > balance→"insufficient balance for value".createPopulatesContractAddress— CREATE call setscontractAddressto the actual deployed address (read from the VM, not re-synthesized).returnFullTransactionsShape— verifies both response shapes, deterministic hash equality across runs.traceTrc10TopLevelCall— owner sends 50 units of token 1000001 to the pre-deployed contract; verifies all four topics + data; owner on-chain TRC-10 balance unchanged.traceTrc10MixedWithTrx— single call with bothvalue: "0x64"andtokenValue: "0x32"to an "accept-anything" sink contract; both synthetic logs emitted in TRX-first order.buffering_dropsTokenTransferOnRevertFrame— direct buffer exercise:revertFramedrops a bufferedonTokenTransfer.Manual Testing
Launched Nile testnet node with changes and run multiple commands with different setups:
Requests and Responses are long, so hide under spoiler