Skip to content
This repository has been archived by the owner on Jul 5, 2024. It is now read-only.

spec for opcode COINBASE #83

Merged
merged 10 commits into from
Jan 12, 2022

Conversation

scroll-dev
Copy link
Contributor

No description provided.

Copy link
Collaborator

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

LGTM, added some nitpicks

specs/opcode/41COINBASE.md Outdated Show resolved Hide resolved
tests/evm/test_coinbase.py Outdated Show resolved Hide resolved
@@ -9,7 +9,7 @@ class Block:
coinbase: U160
gas_limit: U64
block_number: U256
time: U256
time: U64
Copy link
Collaborator

@han0110 han0110 Jan 5, 2022

Choose a reason for hiding this comment

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

Using u64 makes more sense after I saw the actual date of the max uint64 as unix time. Would you mind update the table_assignments of it to use raw value self.time instead of random liinear combination? (in line 45)

Let's discuss how they should exist in table in the #75 thread.

src/zkevm_specs/evm/execution/__init__.py Show resolved Hide resolved
Co-authored-by: Chih Cheng Liang <chihchengliang@gmail.com>
@CPerezz
Copy link
Member

CPerezz commented Jan 10, 2022

Should we merge this @han0110 ?

tests/evm/test_coinbase.py Outdated Show resolved Hide resolved
src/zkevm_specs/evm/execution/block_coinbase.py Outdated Show resolved Hide resolved
@han0110 han0110 merged commit 2fd3a29 into privacy-scaling-explorations:master Jan 12, 2022
@ed255 ed255 mentioned this pull request Jan 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants