fix(evm): only treat 3-topic logs as in-contract transfers (follow-up to #4844)#4852
Open
guo wants to merge 1 commit into
Open
fix(evm): only treat 3-topic logs as in-contract transfers (follow-up to #4844)#4852guo wants to merge 1 commit into
guo wants to merge 1 commit into
Conversation
AddLog still called log.Panic("Invalid in contract transfer topics") when a
log's first topic matched the in-contract-transfer sentinel but the topic
count was not exactly 3. _inContractTransfer is the all-zero topic and log
topics are EVM-controlled, so a 1- or 2-topic log with an all-zero first
topic reached that branch and panicked. #4844 added a len(topics) > 0 guard
that removed the empty-topic index-out-of-range, but left the len != 3 panic
on the 1-/2-topic shape.
Genuine in-contract transfers are always emitted with exactly three topics
(type, from, to), so gate the special-casing on len(topics) == 3; any other
shape is an ordinary log and is appended as such. This removes the remaining
panic without changing behavior for real in-contract-transfer logs or for
any normal log.
Add a regression test covering both the malformed-shape and well-formed paths.
|
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
1 participant
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.
Summary
Follow-up hardening to #4844.
StateDBAdapter.AddLogstill callslog.Panic("Invalid in contract transfer topics")when a log's first topicequals the in-contract-transfer sentinel but the topic count is not exactly 3.
_inContractTransferishash.BytesToHash256([]byte{byte(IN_CONTRACT_TRANSFER)}),i.e. the all-zero topic, and log topics are EVM-controlled. #4844 added a
len(topics) > 0guard that removed the empty-topic (LOG0) index-out-of-range,but a 1- or 2-topic log whose first topic is all-zero still enters the branch and
hits the
len != 3panic.Genuine in-contract transfers are always emitted by the protocol with exactly
three topics (type, from, to), so the correct guard is to enter the special
handling only when
len(topics) == 3. Any other shape is an ordinary log and isappended as such. This removes the remaining panic without changing behavior for
real in-contract-transfer logs or for any normal log (real event signatures are
non-zero keccak hashes, so honest traffic never carries an all-zero first topic).
Change
AddLog: gate the in-contract-transfer handling onlen(topics) == 3 && topics[0] == _inContractTransfer; drop the now-deadlen != 3panic. Malformed shapes fall through to the normal log append.Test
TestAddLogInContractTransferTopicShape: assertsAddLogdoes not panicon a 1-topic log whose topic0 is the sentinel (recorded as a normal log), and
that a well-formed 3-topic in-contract transfer is still recorded as a
transaction log with the correct sender/recipient.
go test ./action/protocol/execution/evm/ -race— full package passes.🤖 Generated with Claude Code