fix: fall back to traces when a creation tx creates the verified contract internally - #2946
fix: fall back to traces when a creation tx creates the verified contract internally#2946marcocastignoli wants to merge 3 commits into
Conversation
…ract internally A contract-creation tx can also create other contracts via internal CREATEs (e.g. a token constructor calling a factory that deploys an LP pair). getContractCreationBytecodeAndReceipt assumed a non-null receipt.contractAddress meant the verified contract was the directly deployed one, and threw on mismatch. Now it falls back to fetching traces, which already verify the tx created the contract at the verified address. Fixes #2932 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kuzdogan
left a comment
There was a problem hiding this comment.
The fix in getContractCreationBytecodeAndReceipt is correct, and the #887 protection holds through the trace helpers. Two related gaps stay open on the server side. Both are pre-existing and out of scope here, but issue #2932 is not fully closed by this PR alone.
1. The server's own creation-tx search still rejects a constructor-created contract. When the request has no hash and no explorer API gives a result, getCreatorTx falls back to findContractCreationTxByBinarySearch in services/server/src/server/services/utils/contract-creation-util.ts. That search finds the creation block, loops over the deploy transactions (tx.to === null), and accepts a tx only when the receipt names the address:
if (
receipt.contractAddress?.toLowerCase() ===
contractAddress.toLowerCase()
) {For the issue's pair contract, the receipt names the token. So the search skips the outer tx and returns null. The pair gets a runtime-only match. This is the same assumption that this PR relaxes in getContractCreationBytecodeAndReceipt. On a chain with traceSupport, the search could use getCreatedAddressesFromBlockTraces (the monitor already uses it in ChainMonitor.ts), which also finds factory-created contracts.
2. Similarity verification stores a hash that was just rejected. In resolveSimilarityCreationData in VerificationService.ts, a hash arrives from the request body or from getCreatorTx. If getContractCreationBytecodeAndReceipt throws because the tx did not create this contract, the catch block returns the hash anyway:
} catch (error: any) {
logger.debug("Failed to fetch creation data for similarity verification", { ... });
return { creationTransactionHash: resolvedCreatorTxHash };
}The mock chain in _verifySimilarity never throws, so Verification keeps the hash, and database-util.ts stores it as contract_deployments.transaction_hash with deployer and block_number empty. Verification.ts clears the hash in the same situation, so the two callers disagree. Returning {} from the catch block, in the same way as the no-hash case, would align them.
Posted with Claude Code
| // (contractAddress === null), or the tx deployed another contract whose constructor | ||
| // created this one (contractAddress set but different, https://github.com/argotorg/sourcify/issues/2932). | ||
| // Both need traces, which also check that the tx actually created this contract | ||
| // and not a random one (https://github.com/argotorg/sourcify/issues/887). |
There was a problem hiding this comment.
After this change, a receipt mismatch also goes to the trace helpers. So more kinds of traces reach them. One kind is a constructor that runs a CREATE that reverts, before the CREATE that makes the verified contract.
Both helpers read a field with no null guard:
// parity: extractCreationBytecodeFromParityTraceProvider
(trace.result.address as string).toLowerCase() === address.toLowerCase()
// and in its error message: createTraces.map((t) => t.result.address)
// geth: extractCreationBytecodeFromGethTraceProvider
(createCall) => createCall.to.toLowerCase() === address.toLowerCase()Current mainnet nodes do not cause a crash here. I checked tx 0x95daecc900beafa524fd1b2119f1acac0e7de773e4164cf44bfd7bf876ab16af (block 25482529). Its constructor runs a CREATE2 that reverts. Geth callTracer returns to: 0x000…000 and error for the failed frame. The Parity-format node behind dRPC keeps result (with code: "0x") and sets error: "Reverted".
But OpenEthereum-era nodes omit result on a failed create. The sibling function extractCreatedAddressesFromParityTraceProvider already guards !trace.result || !trace.result.address. So the code base treats that shape as possible.
If a node returns that shape, this happens:
.findthrowsTypeErroron the failed frame, before it reaches the matching frame.getRpcDataViaTraceTypeconverts the error intotryNext.- Every RPC fails in the same way.
- The caller only sees "All RPCs failed or are blocked". That looks like an outage.
A cheap fix is optional chaining in both helpers and in the error message: trace.result?.address, t.result?.address, createCall.to?.toLowerCase(). This is pre-existing, so a follow-up PR is also fine.
Posted with Claude Code
| // (contractAddress === null), or the tx deployed another contract whose constructor | ||
| // created this one (contractAddress set but different, https://github.com/argotorg/sourcify/issues/2932). | ||
| // Both need traces, which also check that the tx actually created this contract | ||
| // and not a random one (https://github.com/argotorg/sourcify/issues/887). |
There was a problem hiding this comment.
Related: the trace helpers do not read the error field. So they treat a reverted create as a real one, if the node reports an address for it.
Live example: mainnet tx 0x95daec…16af (block 25482529). It is a contract-creation tx. Its constructor runs a CREATE2 that reverts.
Geth debug_traceTransaction with callTracer (QuickNode), the failed frame:
{
"type": "CREATE2",
"from": "0x9d7f372fb27502d7a8…",
"to": "0x0000000000000000000000000000000000000000",
"input": "0x608060405260405161…",
"error": "execution reverted",
"gas": "0x8194",
"gasUsed": "0x4e5d"
}Parity-format trace_transaction (dRPC, a Reth/Erigon-style node), the same failed create:
{
"type": "create",
"action": { "from": "0x9d7f372fb27502d7a8…", "creationMethod": "create2", "init": "0x608060405260405161…" },
"result": { "address": "0x9b4bded299ea8a9da0…", "code": "0x", "gasUsed": "0x4e5d" },
"error": "Reverted",
"traceAddress": [0]
}I checked all 5 frames in each trace. The 3 successful frames have no error key at all. The 2 failed frames have error set. So if (trace.error) is a correct "failed" check for these two node types.
What goes wrong:
extractCreationBytecodeFromParityTraceProvidermatches onresult.addressonly. If the verified address is the would-be address of a failed create, it returns theaction.initof the failed attempt. That address gets code only if a laterCREATE2with the same factory, salt, and init code succeeds. Then the bytecode is correct, but the stored creation tx hash and block point to a tx that did not create the contract.extractCreatedAddressesFromParityTraceProvider(the block scan of the binary search) records the failed tx as the creator of that address. It guards!trace.result, but noterror.- The Geth path is safe only by chance:
tois the zero address, which never matches.
Fix: skip create frames that have error, in both Parity helpers and in findCreateInDebugTraceTransactionCalls. Two notes:
- A create under a parent frame that reverted has no
errorof its own (I saw this in block 25747543). A full check must also look at the parent frames. - Old OpenEthereum nodes omit
resulton a failed create. Addtrace.result?.addressandcreateCall.to?.toLowerCase()as well, or.findthrowsTypeErrorand the lookup fails with "All RPCs failed".
Posted with Claude Code
| // created this one (contractAddress set but different, https://github.com/argotorg/sourcify/issues/2932). | ||
| // Both need traces, which also check that the tx actually created this contract | ||
| // and not a random one (https://github.com/argotorg/sourcify/issues/887). | ||
| if (!this.traceSupport) { |
There was a problem hiding this comment.
Before this change, a wrong creationTransactionHash cost one receipt read. The function then threw "Address … doesn't match the address … created by this transaction". After this change, the mismatch goes to getCreationBytecodeForFactory. That has two effects.
1. One full trace fetch per trace RPC. The trace helper finds no create for the address and throws:
throw new Error(
`Provided tx ${creatorTxHash} does not create the expected contract ${address}. Created contracts by this tx: ...`,
);getRpcDataViaTraceType catches that and returns { tryNext: true }. So executeWithCircuitBreaker asks the next RPC. The trace is immutable, so every RPC gives the same negative answer. The loop only stops after the last trace RPC. creationTransactionHash is unauthenticated input, so a client with many stale hashes multiplies the trace load.
2. The #887 diagnostic is lost. The precise message above only appears at info level, once per RPC. The error that reaches Verification.verify is the generic All RPCs failed or are blocked for getCreationBytecodeForFactory(...). That is the same message as an RPC outage. An operator cannot tell "wrong hash" from "trace RPCs are down". On the Geth side, a plain deploy tx with no inner calls has no calls list, so the helper logs "received empty or malformed response" and blames the RPC for a correct answer.
Suggested fix: a non-retryable error, in the same way as RpcFailure. Add an error class, for example DefinitiveError. Throw it from the "no matching create" branch in both helpers (that branch runs only after the "empty or malformed" check, so tryNext stays correct for incomplete traces). Rethrow it in the two catch blocks of getRpcDataViaTraceType, and in executeWithCircuitBreaker:
} catch (error) {
if (error instanceof DefinitiveError) throw error; // stop, the answer is final
if (error instanceof RpcFailure) { ...; continue; }Then a wrong hash costs one trace fetch, and the caller gets the precise message.
Posted with Claude Code
| }); | ||
|
|
||
| // https://github.com/argotorg/sourcify/issues/2932 | ||
| it('should fall back to traces when the tx deploys another contract whose constructor creates the verified one', async () => { |
There was a problem hiding this comment.
The new tests cover the success path of the trace fallback. One path has no test: the receipt address is different, traceSupport is on, and the trace has no create for the verified address. This is the path that keeps the #887 protection after this change. The expected result is a rejection.
The helper tests ('no create trace is found in parity traces', 'contract address is not found in geth traces') call the helpers directly. Nothing asserts that getContractCreationBytecodeAndReceipt rejects in this case, or how many RPCs it calls. The wrong-hash test in Verification.spec.ts runs on the hardhat chain, which has no traceSupport, so it tests the no-trace branch.
Suggested test: a chain with two trace RPCs, getTxReceipt resolves { contractAddress: '0xtokenAddress' }, send resolves a trace list without '0xpairAddress'. Assert that the call rejects. If the non-retryable error from my other comment is added, also assert that send is called once.
Posted with Claude Code
| ) { | ||
| // The tx deployed this contract directly, so its input data is the creation bytecode | ||
| creationBytecode = creatorTx.data; | ||
| logDebug(`Contract ${address} created with an EOA`); |
There was a problem hiding this comment.
Nit: the two debug messages do not match the branches any more. The first branch means "the tx created this contract directly"; the code does not check the sender, so "EOA" is an inference. The else branch also covers a contract that a constructor created (the new comment above says so), not only a factory. Suggested: created directly by the transaction and not created directly by the transaction, fetching traces. An operator who searches the logs for "factory" then does not read a constructor case as a factory deployment.
Posted with Claude Code
Summary
Fixes #2932.
A contract-creation transaction (
to === null) can create more contracts than the one recorded inreceipt.contractAddress: any internalCREATEduring the constructor also deploys a contract under the same tx hash. In the issue's case (tx0x6f3a…1a38on Conflux eSpace), an EOA deploys a token whose constructor calls a DEX factory, which deploys the LP pair — one tx, two contracts.getContractCreationBytecodeAndReceipttreated a non-nullreceipt.contractAddressas "directly deployed, so the verified address must equal it" and threw on mismatch, never reaching the trace fallback. Now the mismatch case falls through togetCreationBytecodeForFactory, and the "EOA shortcut" is only taken when the receipt's address actually matches the verified contract.The protection from #887 (rejecting a tx hash that doesn't create the verified contract) is preserved: the trace path only returns bytecode for a
CREATEwhose result address matches the verified address, and throws otherwise. When the chain has no trace support, an error is thrown naming both possible causes: a wrong tx hash, or an internal creation that can't be checked without traces.Notes
trace_transaction; the token (directly-deployed) path is unchanged.traceSupport, so this contract also needs Add trace support for Conflux eSpace (1030) sourcifyeth/sourcify-chains#128 to be verifiable on sourcify.dev.🤖 Generated with Claude Code