Skip to content
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

chore: add ReentrancySentryOOG for SSTORE #1795

Merged
merged 3 commits into from
Sep 24, 2024
Merged

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented Sep 23, 2024

Currently reth's trace results

Geth returns a special oog error when gasleft in sstore < 2300, https://github.com/ethereum/go-ethereum/blob/564b6161637fe7e7c28e7c10a1f1978a22861bd2/core/vm/gas_table.go#L184-L188,

func gasSStoreEIP2200(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) {
	// If we fail the minimum gas availability invariant, fail (0)
	if contract.Gas <= params.SstoreSentryGasEIP2200 {
		return 0, errors.New("not enough gas for reentrancy sentry")
	}

so the trace result of reth's is differ to geth's, eg this tx: https://etherscan.io/tx/0x87a19142a638f981a923a6b76859cd4a6d443c430435a1aedf5c9cb35c955768

diff as below:
image

Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't strive to port all geth errors as return codes are not part of consensus and it does not make sense.

But I support this PR, as it cleans up sstore_cost and clarifies the logic flow.

One missing thing in comment, but lgtm

@rakita rakita changed the title feat: add ReentrancySentryOOG for SSTORE chore: add ReentrancySentryOOG for SSTORE Sep 23, 2024
Signed-off-by: jsvisa <[email protected]>
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rakita rakita merged commit 5b24f61 into bluealloy:main Sep 24, 2024
27 checks passed
@jsvisa jsvisa deleted the reentrancy-oog branch September 24, 2024 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants