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

Create blockhash_refactoring.md #210

Merged
merged 13 commits into from
Apr 20, 2018
Merged

Create blockhash_refactoring.md #210

merged 13 commits into from
Apr 20, 2018

Conversation

vbuterin
Copy link
Contributor

Stores blockhashes in the state, reducing the protocol complexity and the need for client implementation complexity in order to process the BLOCKHASH opcode. Also extends the range of how far back blockhash checking can go, with the side effect of creating direct links between blocks with very distant block numbers, facilitating much more efficient initial light client syncing.

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-X.md

Stores blockhashes in the state, reducing the protocol complexity and the need for client implementation complexity in order to process the BLOCKHASH opcode. Also extends the range of how far back blockhash checking can go, with the side effect of creating direct links between blocks with very distant block numbers, facilitating much more efficient initial light client syncing.
@wanderer
Copy link
Member

whats SUPERUSER?

@vbuterin
Copy link
Contributor Author

Perhaps SYSTEM would be a better term. It's an account that represents messages sent "by the protocol", that system contracts might assign special privileges to.

@Smithgift
Copy link

I suggest SYSTEM as well, because SUPERUSER sounds like there's an actual user responsible.

@Arachnid
Copy link
Contributor

Please renumber as eip-210 and we will merge this as a draft.

else:
if ~calldataload(0) >= block.number or ~calldataload(0) < {METROPOLIS_FORK_BLKNUM}:
return 0
if block.number - ~calldataload(0) >= 256:
Copy link

Choose a reason for hiding this comment

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

Should be if block.number - ~calldataload(0) < 256 probably?

@arkpar
Copy link

arkpar commented Apr 25, 2017

Since BLOCKHASH contract code is added to the state its exact bytecode must be specified.

BLOCKHASH_CONTRACT_CODE is set to:

```
0x73fffffffffffffffffffffffffffffffffffffffe33141561006a5760014303600035610100820755610100810715156100455760003561010061010083050761010001555b6201000081071515610064576000356101006201000083050761020001555b5061013e565b4360003512151561008457600060405260206040f361013d565b61010060003543031315156100a857610100600035075460605260206060f361013c565b6101006000350715156100c55762010000600035430313156100c8565b60005b156100ea576101006101006000350507610100015460805260206080f361013b565b620100006000350715156101095763010000006000354303131561010c565b60005b1561012f57610100620100006000350507610200015460a052602060a0f361013a565b600060c052602060c0f35b5b5b5b5b
Copy link
Contributor

Choose a reason for hiding this comment

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

For me this is no-go, because we can't audit the code here. Can we have it in assembly code? If you can't do it ask someone from solidity team to do it. They have nice assembly language that maps 1 to 1 to EVM bytecode.

Copy link
Member

@axic axic May 5, 2017

Choose a reason for hiding this comment

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

Here is a sample (probably broken) implementation, though not sure it is easier to read: https://gist.github.com/axic/c97d104ac63a97049a5d2559ba43460f

Copy link
Member

@pirapira pirapira May 8, 2017

Choose a reason for hiding this comment

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

I need the bytecode. EXTCODECOPY would copy from the bytecode. It's also good if some code is attached in some other languages, with a link to a specific version of a compiler/assembler.

* `VALUE`: 0
* `DATA`: 32 byte zero-byte-leftpadded integer representing the stack argument with which the opcode was called

Also, the gas cost is increased from 20 to 800 to reflect the higher costs of processing the algorithm in the contract code.
Copy link
Member

@axic axic May 4, 2017

Choose a reason for hiding this comment

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

The serpent code below seems to take more than 800 gas for reading.

Copy link
Member

Choose a reason for hiding this comment

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

I think it would make sense setting the cost to the upper bound of what the read actually costs.

If the current proposed rules make that upper bound very large and therefore pose an unrealistically high cost for the opcode, then I would suggest:

  • have a contract which stores less and has a lower higher bound or
  • limiting the parameter to 256 (as currently) for the opcode

Copy link
Member

Choose a reason for hiding this comment

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

Does the gas cost increase from METROPOLIS_FORK_BLKNUM or METROPOLIS_FORK_BLKNUM + 256?

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer the increase at METROPOLIS_FORK_BLKNUM , as all previous cost changes happened at the fork block

@axic
Copy link
Member

axic commented May 4, 2017

Is there any reasoning for the rules of storing the block hashes?

Would a more simple rolling buffer of a fixed item count be more safe as a first milestone and introducing a more sophisticated contract later?


* `METROPOLIS_FORK_BLKNUM`: TBD
* `SUPER_USER`: 2**160 - 2
* `BLOCKHASH_CONTRACT_ADDR`: 0xf0 (ie. 240)
Copy link
Member

Choose a reason for hiding this comment

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

Can this be a continuous address or is there a special reason for using 0xf0?

Copy link
Member

Choose a reason for hiding this comment

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

@vbuterin @chfast this should be 0x9 to avoid any holes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I asked about this before. The answer was this is not regular precompiled contract, because you deploy real bytecode. I think it make sense, because from EVM you don't need additional check "if precompiled" to use this contract.

However, I'd use address of 0x100 to leave the lower byte for precompiled contracts only.

@chfast
Copy link
Contributor

chfast commented May 5, 2017

Because of the incoming RETURNDATA changes #211, we have to decide what the contract returns in case of invalid block number or some other exceptional condition (e.g. value transfer). Options are:

  • return "null" hash (32 bytes),
  • stop (0 bytes),
  • fail (OOG flag, 0 bytes).

@chfast
Copy link
Contributor

chfast commented May 5, 2017

I can start writing tests for the contract if you are happy with using pyethereum for it.

If `block.number >= METROPOLIS_FORK_BLKNUM`, then when processing a block, before processing any transactions execute a call with the parameters:

* `SENDER`: SUPER_USER
* `GAS`: 1000000
Copy link
Member

@pirapira pirapira May 8, 2017

Choose a reason for hiding this comment

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

What is the gas price and the nonce of this call transaction?

* `GAS`: 1000000
* `TO`: BLOCKHASH_CONTRACT_ADDR
* `VALUE`: 0
* `DATA`: 32 byte zero-byte-leftpadded integer representing the stack argument with which the opcode was called
Copy link
Member

@pirapira pirapira May 8, 2017

Choose a reason for hiding this comment

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

What is the gas price and the nonce of this call? I guess this is just a message call and not a separate transaction.

If `block.number >= METROPOLIS_FORK_BLKNUM + 256`, then the BLOCKHASH opcode instead returns the result of executing a call with the parameters:

* `SENDER`: <account from which the opcode was called>
* `GAS`: 1000000
Copy link
Member

@pirapira pirapira May 8, 2017

Choose a reason for hiding this comment

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

What happens when 1000000 is more than "all-but-64th" of the remaining gas? A usual CALL instruction would lower the number automatically.

* `VALUE`: 0
* `DATA`: <32 bytes corresponding to the block's prevhash>

If `block.number >= METROPOLIS_FORK_BLKNUM + 256`, then the BLOCKHASH opcode instead returns the result of executing a call with the parameters:
Copy link
Member

Choose a reason for hiding this comment

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

Is the call-stack depth limit of 1024 still relevant?

Copy link
Member

Choose a reason for hiding this comment

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

This question was answered in #210 (comment)


If `block.number >= METROPOLIS_FORK_BLKNUM + 256`, then the BLOCKHASH opcode instead returns the result of executing a call with the parameters:

* `SENDER`: <account from which the opcode was called>
Copy link
Member

Choose a reason for hiding this comment

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

Github Markdown renderer doesn't show this text in angle brackets for some reason

@vbuterin
Copy link
Contributor Author

vbuterin commented May 10, 2017

In the last call a decision was made that the BLOCKHASH opcode (the opcode, not directly calling the contract!) only works for the last 256 blocks as before. Hence, there are now three equivalent ways to implement BLOCKHASH:

  1. If block.number - 256 <= arg <= block.number - 1, then call 0x000...00f0 and return the result (@pirapira ignoring parent gas limits and call depth limits)
  2. If block.number - 256 <= arg <= block.number - 1, then look through the actual blockchain
  3. If block.number - 256 <= arg <= block.number - 1, then grab the result from the correct storage location in the storage of 0x000...00f0

The fact that (1) and (3) are possible is nice, because it removes the long block header history from the "de-facto state"; now, only knowing the last 8 headers and the state is sufficient for fully validating and processing blocks. The fact that (2) is possible is nice, as it means that there is low effort required for existing clients to switch over.

@chfast
Copy link
Contributor

chfast commented Jan 30, 2018

Summary of my comments:

  1. The code still have a bug: the input argument is treated as signed integer, and some comparisons does not work as expected.
  2. I'd like to write unit tests for the contract. So far I used pyethereum in EIP-210: Add BLOCKHASH contract tests #641. I'm open for a suggestions for other test framework. It must allow to rewind blocks quickly.
  3. I'd like to implement this contract in IULIA to estimate the lower boundary of the gas consumption. I believe we don't have to subsidy the reading from the contract.

@AlexeyAkhunov
Copy link
Contributor

Have you thought about back-filling the old blockhashes in a second hard-fork? In the first hard-fork, a method is added for the SYSTEM user to backfill the historical hashes, and then in the second hard-fork, the actual back-filling transactions are inserted.

Another solution to the back-filling is to allow anyone to submit the block headers so that the contract can hash them, check the block hash and populate the prev hash from the header. The cost is in historical block headers effectively repeated in the block bodies, which would currently be ~500M extra disk space + gas to populate all those headers. This solution does not require hard fork though

@chfast
Copy link
Contributor

chfast commented Mar 1, 2018

Please merge this as Draft with number 210 as agreed in All Core Devs Meeting #34.

@Arachnid
Copy link
Contributor

This is a courtesy notice to let you know that the format for EIPs has been modified slightly. If you want your draft merged, you will need to make some small changes to how your EIP is formatted:

  • Frontmatter is now contained between lines with only a triple dash ('---')
  • Headers in the frontmatter are now lowercase.

If your PR is editing an existing EIP rather than creating a new one, this has already been done for you, and you need only rebase your PR.

In addition, a continuous build has been setup, which will check your PR against the rules for EIP formatting automatically once you update your PR. This build ensures all required headers are present, as well as performing a number of other checks.

Please rebase your PR against the latest master, and edit your PR to use the above format for frontmatter. For convenience, here's a sample header you can copy and adapt:

---
eip: <num>
title: <title>
author: <author>
type: [Standards Track|Informational|Meta]
category: [Core|Networking|Interface|ERC] (for type: Standards Track only)
status: Draft
created: <date>
---

@Arachnid Arachnid merged commit dc16346 into master Apr 20, 2018
@holiman
Copy link
Contributor

holiman commented Apr 24, 2018

Re-posting here (from gitter AllCoreDev) -- because I'm not sure where the discussion should go

  1. Wouldn't it make sense to rewrite it using the new shift opcodes?
  2. Is the gas consumption counted, in any way. For example, if block's gas is <1M (private chains) -- we still run the setter, correct?
  3. When we do the opcode -> CALL transition (whenever someone uses the opcode BLOCKHASH), the cost is 800 gas. But in the CALL context, we suddenly have 1M gas. Is this required? What is the worst-case gas usage of executing it (as non-SUPERUSER) ? ( And similarly, I assume the 1M is a temporary thing which doesn't need to fit within block gas limits)

Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request May 2, 2018
* Create blockhash_refactoring.md

Stores blockhashes in the state, reducing the protocol complexity and the need for client implementation complexity in order to process the BLOCKHASH opcode. Also extends the range of how far back blockhash checking can go, with the side effect of creating direct links between blocks with very distant block numbers, facilitating much more efficient initial light client syncing.

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Update blockhash_refactoring.md

* Rename blockhash_refactoring.md to eip-210.md
@sorpaas
Copy link
Contributor

sorpaas commented Aug 3, 2018

The new EIP-210 code would return out of gas if the current block number is 1:

  • bn equals 0.
  • if ~mod(bn, 256) would be false.
  • bn = ~div(bn, 256) would get us 0 again.
  • Infinite loop.

Is this intentional?

@chfast
Copy link
Contributor

chfast commented Aug 3, 2018

No. #1094.

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.

None yet