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: switch gas check order in blake2 precompile #1718

Merged
merged 1 commit into from
Aug 26, 2024

Conversation

pcy190
Copy link
Contributor

@pcy190 pcy190 commented Aug 26, 2024

Go-ethereum always check gas before checking the detailed flag in [1]:

func RunPrecompiledContract(p PrecompiledContract, input []byte, suppliedGas uint64, logger *tracing.Hooks) (ret []byte, remainingGas uint64, err error) {
	gasCost := p.RequiredGas(input)
	if suppliedGas < gasCost {
		return nil, 0, ErrOutOfGas
	}
	if logger != nil && logger.OnGasChange != nil {
		logger.OnGasChange(suppliedGas, suppliedGas-gasCost, tracing.GasChangeCallPrecompiledContract)
	}
	suppliedGas -= gasCost
	output, err := p.Run(input)
	return output, suppliedGas, err
}

[1]https://github.com/ethereum/go-ethereum/blob/941ae33d7e0b36afc2f8551884f555d963de7c6b/core/vm/contracts.go#L190-L206

Therefore, when the final flag is invalid (neither 1 nor 0) in blake2 precompile, and the gas limit is below the minimum requirement, the behavior differs between revm and go-ethereum.

Copy link
Collaborator

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

i don't think this matters much because all error cases are the same? but it's probably fine

@@ -17,19 +17,19 @@ pub fn run(input: &Bytes, gas_limit: u64) -> PrecompileResult {
return Err(Error::Blake2WrongLength.into());
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

technically this check also would go below gas check, and then gas check would have to handle empty input...

@rakita rakita changed the title fix gas check order in blake2 precompile chore: switch gas check order in blake2 precompile Aug 26, 2024
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.

It is a semantic change, lgtm.

Have renamed the issue to chore: as it does not affect consensus between revm and geth.

@rakita rakita merged commit e155d07 into bluealloy:main Aug 26, 2024
27 checks passed
This was referenced Aug 26, 2024
lightsing added a commit to scroll-tech/revm that referenced this pull request Sep 3, 2024
* chore(eof): simplify magic checks (bluealloy#1633)

* perf(eof): avoid some allocations (bluealloy#1632)

* perf(eof): avoid some allocations

* Update crates/primitives/src/bytecode/eof.rs

* chore: fix some typos & remove useless Arc::clone (bluealloy#1621)

* chore: fix some typos

* chore: remove useless Arc::clone

* refactor: use `is_zero` for `U256` and `B256` (bluealloy#1638)

* refactor: use is_zero for U256 and B256

* fmt

* Update crates/interpreter/src/instructions/arithmetic.rs

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>

* fix deref

* move import

---------

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>

* feat(interpreter): derive traits on FunctionStack (bluealloy#1640)

* chore: Renamed some city name (bluealloy#1645)

* perf: avoid cloning original_bytes (bluealloy#1646)

* fix(eof): deny static context in EOFCREATE (bluealloy#1644)

* feat: use batch bn256 pair operation (bluealloy#1643)

* feat: use batch bn256 pair operation

We are currently not taking advantage of the batch pair operation from
the `bn` library for the pairing check precompile.

This yields a ~27% speedup on the existing bench:
```
Crypto Precompile benchmarks/precompile bench | ecpairing precompile
                        time:   [2.2389 ms 2.2441 ms 2.2495 ms]
                        change: [-27.689% -27.469% -27.227%] (p = 0.00 < 0.05)
                        Performance has improved.
```

* use with_capacity

* import vec

* feat(EOF): implement std::error::Error trait for EofValidationError and EofError (bluealloy#1649)

* feat: implement Error trait for EofValidationError

* feat: implement Error trait for EofError

* fix: remove unused import

* fix: remove format macro

* chore(deps): bump thiserror from 1.0.62 to 1.0.63 (bluealloy#1651)

Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.62 to 1.0.63.
- [Release notes](https://github.com/dtolnay/thiserror/releases)
- [Commits](dtolnay/thiserror@1.0.62...1.0.63)

---
updated-dependencies:
- dependency-name: thiserror
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump tokio from 1.38.0 to 1.38.1 (bluealloy#1650)

Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.38.0 to 1.38.1.
- [Release notes](https://github.com/tokio-rs/tokio/releases)
- [Commits](tokio-rs/tokio@tokio-1.38.0...tokio-1.38.1)

---
updated-dependencies:
- dependency-name: tokio
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(EOF): Overflow on num_sections (bluealloy#1656)

* fix(EOF): Overflow on num_sections

* fix test

* fmt/clippy

* feat(EOF): EOF Validation add code type and sub container tracker (bluealloy#1648)

* feat(EOF): EOF Validation add code type and sub container tracker

* fix

* omit tests

* fix some things, bump test suite

* fix(EOF): Overflow on num_sections

* cleanup fmt

* clippy

* fix tests

* Run EOF validation tests

* fix(statetest): Add back Merge spec (bluealloy#1658)

* fix(EOF): Validate code access in stack (bluealloy#1659)

* wip: test

* fix(EOF): Validate code access in stack

* add code access

* feat(EOF): Add EOF validation in revme bytecode cmd (bluealloy#1660)

* chore(clippy): 1.80 rust clippy list paragraph ident (bluealloy#1661)

* feat(EOF): Add non-returning CALLF/JUMPF checks (bluealloy#1663)

* feat(EOF): Add non-returning CALLF/JUMPF checks

* fix tests

* fix(EOF): returning to non-returning jumpf, enable valition error (bluealloy#1664)

* fix: add DATACOPY to OpCode::modifies_memory (bluealloy#1639)

* chore(eof): Add opcodes that expand memory (bluealloy#1665)

* fix(statetest): make bytecode analyzed (bluealloy#1666)

* chore(deps): bump serde_json from 1.0.120 to 1.0.121 (bluealloy#1667)

Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.120 to 1.0.121.
- [Release notes](https://github.com/serde-rs/json/releases)
- [Commits](serde-rs/json@v1.0.120...v1.0.121)

---
updated-dependencies:
- dependency-name: serde_json
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump tokio from 1.38.1 to 1.39.2 (bluealloy#1668)

Bumps [tokio](https://github.com/tokio-rs/tokio) from 1.38.1 to 1.39.2.
- [Release notes](https://github.com/tokio-rs/tokio/releases)
- [Commits](tokio-rs/tokio@tokio-1.38.1...tokio-1.39.2)

---
updated-dependencies:
- dependency-name: tokio
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump blst from 0.3.12 to 0.3.13 (bluealloy#1669)

Bumps [blst](https://github.com/supranational/blst) from 0.3.12 to 0.3.13.
- [Release notes](https://github.com/supranational/blst/releases)
- [Commits](supranational/blst@v0.3.12...v0.3.13)

---
updated-dependencies:
- dependency-name: blst
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump serde_json from 1.0.121 to 1.0.122 (bluealloy#1678)

Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.121 to 1.0.122.
- [Release notes](https://github.com/serde-rs/json/releases)
- [Commits](serde-rs/json@v1.0.121...v1.0.122)

---
updated-dependencies:
- dependency-name: serde_json
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump alloy-eips from 0.2.0 to 0.2.1 (bluealloy#1679)

Bumps [alloy-eips](https://github.com/alloy-rs/alloy) from 0.2.0 to 0.2.1.
- [Release notes](https://github.com/alloy-rs/alloy/releases)
- [Changelog](https://github.com/alloy-rs/alloy/blob/main/CHANGELOG.md)
- [Commits](alloy-rs/alloy@v0.2.0...v0.2.1)

---
updated-dependencies:
- dependency-name: alloy-eips
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump regex from 1.10.5 to 1.10.6 (bluealloy#1682)

Bumps [regex](https://github.com/rust-lang/regex) from 1.10.5 to 1.10.6.
- [Release notes](https://github.com/rust-lang/regex/releases)
- [Changelog](https://github.com/rust-lang/regex/blob/master/CHANGELOG.md)
- [Commits](rust-lang/regex@1.10.5...1.10.6)

---
updated-dependencies:
- dependency-name: regex
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump rstest from 0.21.0 to 0.22.0 (bluealloy#1681)

Bumps [rstest](https://github.com/la10736/rstest) from 0.21.0 to 0.22.0.
- [Release notes](https://github.com/la10736/rstest/releases)
- [Changelog](https://github.com/la10736/rstest/blob/master/CHANGELOG.md)
- [Commits](la10736/rstest@v0.21.0...v0.22.0)

---
updated-dependencies:
- dependency-name: rstest
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Add EOF Layout Fuzz Loop to `revme bytecode` (bluealloy#1677)

* Add EOF Layout Fuzz Loop to `bytecode`

Update `revme` so that when `bytecode` is called without arguments it goes into the standard

* Add size check in fuzzer

* code format

* chore: introduce bytecode initcode/runtime cli flags

* docs: improve `InstructionResult` documentation (bluealloy#1673)

* docs: improve `InstructionResult` documentation

* chore: clean up

* Add OP-Granite hardfork, limiting bn256Pairing input size (bluealloy#1685)

* Add OP-Granite hardfork, limiting bn256Pairing input size

* Move optimism-specific bn128 precompile

* feat: check for typos in CI (bluealloy#1686)

Co-authored-by: adria0 <adria0@nowhere>

* feat(EOF): add evmone test suite (bluealloy#1689)

* feat(EOF): add evmone test suite

* pass all tests

* multiple paths for eof-validations

* path to eest eof validations tests

* feat(EOF): Run EOF tests from eth/tests (bluealloy#1690)

* chore: release (bluealloy#1683)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* bump: tag v41 revm v13.0.0 (bluealloy#1692)

* fix(CI): types check (bluealloy#1693)

* fix(CI): types check

* Typos

* chore(deps): bump alloy-provider from 0.2.0 to 0.2.1 (bluealloy#1680)

Bumps [alloy-provider](https://github.com/alloy-rs/alloy) from 0.2.0 to 0.2.1.
- [Release notes](https://github.com/alloy-rs/alloy/releases)
- [Changelog](https://github.com/alloy-rs/alloy/blob/main/CHANGELOG.md)
- [Commits](alloy-rs/alloy@v0.2.0...v0.2.1)

---
updated-dependencies:
- dependency-name: alloy-provider
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "chore(deps): bump alloy-provider from 0.2.0 to 0.2.1 (bluealloy#1680)" (bluealloy#1696)

This reverts commit 0a5be93.

* chore(deps): bump alloy-transport from 0.2.0 to 0.2.1 (bluealloy#1698)

Bumps [alloy-transport](https://github.com/alloy-rs/alloy) from 0.2.0 to 0.2.1.
- [Release notes](https://github.com/alloy-rs/alloy/releases)
- [Changelog](https://github.com/alloy-rs/alloy/blob/main/CHANGELOG.md)
- [Commits](alloy-rs/alloy@v0.2.0...v0.2.1)

---
updated-dependencies:
- dependency-name: alloy-transport
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): bump enumn from 0.1.13 to 0.1.14 (bluealloy#1701)

Bumps [enumn](https://github.com/dtolnay/enumn) from 0.1.13 to 0.1.14.
- [Release notes](https://github.com/dtolnay/enumn/releases)
- [Commits](dtolnay/enumn@0.1.13...0.1.14)

---
updated-dependencies:
- dependency-name: enumn
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* doc: update some docs related to state (bluealloy#1711)

* chore: clean up some journalstate docs (bluealloy#1712)

* chore(deps): bump bytes from 1.6.1 to 1.7.1 (bluealloy#1700)

Bumps [bytes](https://github.com/tokio-rs/bytes) from 1.6.1 to 1.7.1.
- [Release notes](https://github.com/tokio-rs/bytes/releases)
- [Changelog](https://github.com/tokio-rs/bytes/blob/master/CHANGELOG.md)
- [Commits](tokio-rs/bytes@v1.6.1...v1.7.1)

---
updated-dependencies:
- dependency-name: bytes
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: switch gas check order in blake2 precompile (bluealloy#1718)

* chore(deps): bump serde from 1.0.204 to 1.0.209 (bluealloy#1717)

Bumps [serde](https://github.com/serde-rs/serde) from 1.0.204 to 1.0.209.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.204...v1.0.209)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* docs: fix spelling issues (bluealloy#1715)

* Update memory.md

* Update bits.md

* Update kzg.md

* Update documentation/src/crates/primitives/kzg.md

Co-authored-by: Oliver <onbjerg@users.noreply.github.com>

* Update bits.md

---------

Co-authored-by: Oliver <onbjerg@users.noreply.github.com>

* feat: c-kzg bump, cleanup on kzgsetting (bluealloy#1719)

* chore: bump c-kzg v2.0.0

* feat: cleanup c-kzg kzgsetting

* rm kzg-rs from no_std check

* chore: bump `kzg-rs` version (bluealloy#1726)

* chore: bump `kzg-rs` version

* fix: remove serde req

* ci: check `kzg-rs` in no-std mode

* chore: update kzg-rs dep

* Update .github/workflows/ci.yml

* Update .github/workflows/ci.yml

* chore: cast block number to u64 and not usize (bluealloy#1727)

* fix: cast block number to u64 and not usize

* clippy

* feat(eip7702): Impl newest version of EIP  (bluealloy#1695)

* latest eip7702 wip

* add code loading handler

* WIP adding is_delegate_cold flag

* feat: add StateLoad and Eip7702CodeLoad

* feat: add gas accounting among other things

* clippy,fmt, op test

* path to latest alloy-eips

* comment eip7702 decode tests

* Eip7702 format starts with 0xEF0100

* typo

* fix(eip7702): fix empty or eip7702 code check

* Type Eip7702s to Eip7702

* Corrent comments

* switch new and new_raw Eip7702Bytecode

* propagate last commit

* nit: rename fn

* fix(eip7702): set delegated code on call (bluealloy#1706)

* type change, return eip7702 raw on Bytecode::bytecode

* eip7702 delegation test

* Cleanup, refactor sstore gas calc

* doc

* chore: add AuthList json format

* fix initial eip7702 gas, fix eip7702 refund on revert

* small refactor

* fix refund cnt

* error handling, EIP-3607 fix, wip on auth validity

* add auth validity check, fix EIP-3607 fix

* switch tests

* missing comment

* fix tests

* rm println

* remove skip of required fields

* docs, test meta dat

* chore(deps): bump alloy and primitives (bluealloy#1725)

* latest eip7702 wip

* add code loading handler

* WIP adding is_delegate_cold flag

* feat: add StateLoad and Eip7702CodeLoad

* feat: add gas accounting among other things

* clippy,fmt, op test

* path to latest alloy-eips

* comment eip7702 decode tests

* Eip7702 format starts with 0xEF0100

* typo

* fix(eip7702): fix empty or eip7702 code check

* Type Eip7702s to Eip7702

* Corrent comments

* switch new and new_raw Eip7702Bytecode

* propagate last commit

* nit: rename fn

* fix(eip7702): set delegated code on call (bluealloy#1706)

* type change, return eip7702 raw on Bytecode::bytecode

* eip7702 delegation test

* Cleanup, refactor sstore gas calc

* doc

* chore: add AuthList json format

* chore(deps): bump alloy

* fix initial eip7702 gas, fix eip7702 refund on revert

* small refactor

* fix refund cnt

* error handling, EIP-3607 fix, wip on auth validity

* rm patches

* add auth validity check, fix EIP-3607 fix

* switch tests

* missing comment

* fix tests

* rm println

* remove skip of required fields

* docs, test meta dat

* chore: release (bluealloy#1722)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* bump: main changelog (bluealloy#1730)

* fix

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
Co-authored-by: jakevin <jakevingoo@gmail.com>
Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Co-authored-by: ioterw <iotrwewe12@protonmail.com>
Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>
Co-authored-by: PA <50184410+peyha@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: rakita <rakita@users.noreply.github.com>
Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
Co-authored-by: Danno Ferrin <danno@numisight.com>
Co-authored-by: Léo Vincent <28714795+leovct@users.noreply.github.com>
Co-authored-by: Brian Bland <brian.t.bland@gmail.com>
Co-authored-by: adria0.eth <5526331+adria0@users.noreply.github.com>
Co-authored-by: adria0 <adria0@nowhere>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: James Prestwich <james@prestwi.ch>
Co-authored-by: Oliver <onbjerg@users.noreply.github.com>
Co-authored-by: HAPPY <pcy190@126.com>
Co-authored-by: Elias Rad <146735585+nnsW3@users.noreply.github.com>
Co-authored-by: Bhargav Annem <bhargav.annem@gmail.com>
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.

3 participants