-
Notifications
You must be signed in to change notification settings - Fork 20.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
all: mv loggers to eth/tracers #23892
Conversation
So most of the changes were trivial, only moving stuff around and changing some imports. Only thing was the struct logger's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor comment, otherwise generally LGTM
eth/tracers/logger/logger_test.go
Outdated
contract.Code = []byte{byte(vm.PUSH32)} | ||
one := uint256.NewInt(1).Bytes32() | ||
zero := new(uint256.Int).Bytes32() | ||
contract.Code = append(contract.Code, one[:]...) | ||
contract.Code = append(contract.Code, byte(vm.PUSH32)) | ||
contract.Code = append(contract.Code, zero[:]...) | ||
contract.Code = append(contract.Code, byte(vm.SSTORE)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contract.Code = []byte{byte(vm.PUSH32)} | |
one := uint256.NewInt(1).Bytes32() | |
zero := new(uint256.Int).Bytes32() | |
contract.Code = append(contract.Code, one[:]...) | |
contract.Code = append(contract.Code, byte(vm.PUSH32)) | |
contract.Code = append(contract.Code, zero[:]...) | |
contract.Code = append(contract.Code, byte(vm.SSTORE)) | |
contract.Code = []byte{ | |
byte(vm.PUSH1), 0x1, | |
byte(vm.PUSH1), 0x0, | |
byte(vm.SSTORE), | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could even be further simplified by changing to sstore(1,1)
:
contract.Code = []byte{
byte(vm.PUSH1), 0x1, byte(vm.DUP1), byte(vm.SSTORE),
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, that's what I had first but it was failing, as I later found out, for another reason. Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
* all: mv loggers to eth/tracers * core/vm: minor * eth/tracers: tmp comment out testStoreCapture * eth/tracers: uncomment and fix logger test * eth/tracers: simplify test * core/vm: re-add license * core/vm: minor * rename LogConfig to Config
* all: mv loggers to eth/tracers * core/vm: minor * eth/tracers: tmp comment out testStoreCapture * eth/tracers: uncomment and fix logger test * eth/tracers: simplify test * core/vm: re-add license * core/vm: minor * rename LogConfig to Config
Mostly creating the PR so i can see the failures through CI xD