feat: add inline simulation using debug_traceCall #877
Conversation
kant777
force-pushed
the
feat/inline-simulation
branch
from
d362dac to
e638981
Compare
|
kant777
force-pushed
the
feat/inline-simulation
branch
9 times, most recently
from
36965f5 to
b1e9636
Compare
|
kant777
force-pushed
the
feat/inline-simulation
branch
2 times, most recently
from
cecf50c to
0e15f77
Compare
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
CI failure due to protobuf parity. Not sure why this is happening since you havent changed the protobuf generated files. Best to generate them once more to see if there are any changes.
cd p2p
make bufgen
| type SwapDetector struct{} | ||
|
|
||
| // NewSwapDetector creates a new swap detector | ||
| func NewSwapDetector() *SwapDetector { |
There was a problem hiding this comment.
This is not needed since we are not having any shared state. Better to just have package level functions:
func DetectFromLogs, it can use the pkg level constants.
iowar
left a comment
iowar
left a comment
There was a problem hiding this comment.
If possible.. I’d prefer to offload most of the call load to eth_simulateV1, if the RPC supports it, because debug_traceCall puts a significant load on the RPC.. especially with a high number of swaps.. and increases response times.. additionally within rethsim, debug_traceCall was being invoked with separate logic for some edge cases.. however, for the first working version, we can stick with debug_traceCall if performance won’t be a concern.. also, I’d suggest testing this PR in real time with actual fastrpc transactions for a while
| ) | ||
|
|
||
| // Known swap event signatures (topic0) | ||
| var swapEventSignatures = map[common.Hash]string{ |
There was a problem hiding this comment.
let’s move the signatures detected in rethsim here, and the ones that haven’t been added to rethsim yet can stay here
|
|
||
| // DetectFromLogs checks if logs contain swap events | ||
| func (d *SwapDetector) DetectFromLogs(logs []TraceLog) (bool, []string) { | ||
| var swapKinds []string |
There was a problem hiding this comment.
suggestion: aligning to rethsim naming or adding a mapping
| map[string]interface{}{ | ||
| "tracer": "callTracer", | ||
| "tracerConfig": map[string]interface{}{ | ||
| "withLog": true, |
There was a problem hiding this comment.
It makes more sense to try to capture the revert reason on the trace side by using enableReturnData: true.. If the Output field in the callTracer result is empty.. decodeRevertFromTrace can be used as a fallback
|
|
||
| // Validate trace response - a valid trace always has non-zero GasUsed | ||
| // (at minimum, intrinsic gas of 21000 is consumed) | ||
| if result.GasUsed == "" || result.GasUsed == "0x" { |
There was a problem hiding this comment.
suggestion: use hexutil.DecodeUint64(result.GasUsed) and validate non zero
| } | ||
|
|
||
| // Get sender | ||
| signer := types.LatestSignerForChainID(tx.ChainId()) |
There was a problem hiding this comment.
mostly correct.. but it sometimes fails.. rethsim does its own transaction parsing (from raw data to a call object)
There was a problem hiding this comment.
looked into rethsim code and I see that your are doing the manual decoding but It looks like all of those cases are handled in types.LatestSignerForChainID(tx.ChainId()). If not, highlight the ones it doesnt handle?
| } | ||
|
|
||
| // Check for inner call errors (recursive) | ||
| if innerErr := findInnerCallError(result); innerErr != "" { |
There was a problem hiding this comment.
suggestion: include the failing call path (to | type) in the inner revert error
|
|
||
| // Also check if destination is a known router | ||
| if !isSwap { | ||
| isSwap, _ = s.swapDetector.DetectFromDestination(tx.To()) |
There was a problem hiding this comment.
- expand swap topics (port from rethsim)
- return swapKinds, not only isSwap
- don’t rely on dex router.. detect via event topic signatures only
| ] | ||
| }` | ||
|
|
||
| func TestInlineSimulator(t *testing.T) { |
There was a problem hiding this comment.
It’s important to include at least one real signed transaction
| } | ||
|
|
||
| // Known DEX router addresses (Ethereum mainnet) | ||
| var knownRouters = map[common.Address]string{ |
There was a problem hiding this comment.
we don’t really need router addresses.. the signatures are sufficient
kant777
force-pushed
the
feat/inline-simulation
branch
from
5c1a3d2 to
6d0898d
Compare
- Add InlineSimulator using debug_traceCall with callTracer - Support pending state simulation (required for preconf-rpc) - Add enableReturnData for better revert capture - Port all 15 swap signatures from rethsim - Detect swaps via event topic signatures only - Support multiple RPC endpoints with fallback on 5xx/429/network errors - Add --use-inline-simulation feature flag (default: false) - Include call path (to, type) in inner revert errors
kant777
force-pushed
the
feat/inline-simulation
branch
from
6d0898d to
758160b
Compare
- Use eth_simulateV1 as the primary simulation method for better performance - Fall back to debug_traceCall when eth_simulateV1 is not supported by the RPC - eth_simulateV1 is lighter and reduces load on RPC providers - debug_traceCall is still used for edge cases or when eth_simulateV1 is unavailable - Add SimulateV1CallResult, SimulateError, SimulateV1Block structs for eth_simulateV1 response - Add isMethodNotSupported() to detect unsupported method errors - Update tests to cover both eth_simulateV1 and fallback scenarios Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ndpoint fallback Transaction reverts and invalid response errors were not wrapped in NonRetryableError, causing the code to unnecessarily try all fallback endpoints when a transaction reverts. This wasted resources and delayed error responses. Changes: - Wrap revert errors in NonRetryableError in both eth_simulateV1 and debug_traceCall - Wrap empty/invalid response errors in NonRetryableError - Update shouldFallback to check for NonRetryableError first - Add clarifying comments about fallback behavior This ensures reverts fail fast instead of trying all endpoints. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
yes debug_traceCall is heavy so changed the code to use that only as a fallback. eth_simulateV1 will be the first choice |
- Remove redundant comments that just repeat what the code does - Keep comments where they explain why, not what - Make comments more concise and natural - Simplify code structure in a few places Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
aloknerurkar
left a comment
aloknerurkar
left a comment
There was a problem hiding this comment.
From a go perspective the changes look good. But I havent used the eth_simulateV1 endpoint. So as long as that and the topics needed are verified we can merge. I am approving from my side.
Adds support for using standard Ethereum RPC providers (Alchemy, Erigon) for transaction simulation via debug_traceCall, as an alternative to the external rethsim endpoint.
Add InlineSimulator using debug_traceCall with callTracer
Add SwapDetector for detecting swaps via event signatures and known routers
Auto-detect simulation mode based on URL pattern (contains /rethsim or not)
Support all transaction types including EIP-4844 blob transactions
Add Prometheus metrics and proper resource cleanup
I have added tests that prove my fix is effective or that my feature works