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.

Sign up for GitHub

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

Jump to bottom

EIP-96: Avoid storing duplicated hashes #643

Closed
wants to merge 4 commits into from
Closed

EIP-96: Avoid storing duplicated hashes #643

wants to merge 4 commits into from

Conversation

chfast
Copy link
Member

@chfast chfast commented Jun 5, 2017 •
edited
Loading

Parent EIP: #210.

This avoids storing duplicated hashes in EIP-96 BLOCKHASH contract by moving to-be-replaced entries from lower levels to higher levels. This also increases scope from 16777216 to 16777216 + 65536 + 256.

@chfast chfast mentioned this pull request Jun 5, 2017
Merged
pirapira
pirapira reviewed Jun 6, 2017
View reviewed changes
EIPS/blockhash_refactoring.md Outdated
Let `n` be the current block number.
Let `p = n - 1` be the previous block number.
Let `h` be the block hash of the previous `p` block.
Let `B` be the base -- the number of records keep on every level.
Copy link
Member

Choose a reason for hiding this comment

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

keep -> kept.

Arachnid
Arachnid reviewed Jun 8, 2017
View reviewed changes
EIPS/blockhash_refactoring.md Outdated
Let `h` be the block hash of the previous `p` block.
Let `B` be the base -- the number of records kept on every level.

The levels are numerated by `k`, starting from 0. A level block hash period
Copy link
Contributor

Choose a reason for hiding this comment

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

enumerated? numbered? 'numerated' isn't a word.

EIPS/blockhash_refactoring.md Outdated
Let `B` be the base -- the number of records kept on every level.

The levels are numerated by `k`, starting from 0. A level block hash period
is `B<sup>k</sup>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the phrase "A level block hash period is B^k". Can you clarify?

@chfast chfast force-pushed the eip96-noduplicates branch from 303ce96 to 2b785df Compare June 9, 2017 15:15
chfast added 2 commits June 9, 2017 22:15
@chfast
EIP-96: Avoid storing duplicated hashes
201262e 
This avoids storing duplicated hashes in EIP-96 BLOCKHASH contract by moving to-be-replaced entries from lower levels to higher levels. This also increases scope from 16777216 to 16777216 + 65536 + 256.
@chfast
EIP-96: Add BLOCKHASH contract tests
04a163c
@chfast chfast force-pushed the eip96-noduplicates branch from 2b785df to 04a163c Compare June 9, 2017 20:28
@chfast
Copy link
Member Author

chfast commented Jul 28, 2017

CC @vbuterin.

@chfast
Copy link
Member Author

chfast commented Aug 10, 2017

Fixed storage update rules!

@chfast chfast mentioned this pull request Aug 11, 2017
Closed
@chfast
Copy link
Member Author

chfast commented Aug 11, 2017

@vbuterin @Arachnid @pirapira @axic Can you give me general feedback about this idea? We have to decide if going with this one or the original design.

After the decision is made I would be able to finish the test set, fix the contract bugs and optimize the contract.

@pirapira
Copy link
Member

I think it's good to avoid duplicates in the storage, but the starting point looks deprecated. It's even better to start from the newer code by @Arachnid and @vbuterin and remove the duplicates in the storage.

@chfast
Copy link
Member Author

chfast commented Aug 11, 2017

@pirapira I never received the newer code. Where can I see it?

@pirapira
Copy link
Member

pirapira commented Aug 12, 2017 •
edited
Loading

I see some code in https://gitter.im/ethereum/AllCoreDevs Jul 14 18:00, 2017. @vbuterin or @Arachnid might have some even newer code.

@pirapira pirapira added the EIP label Dec 1, 2017
@pirapira
Copy link
Member

pirapira commented Jan 3, 2018

What is the status of this PR now?

@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>
---

@axic
Copy link
Member

axic commented Oct 16, 2018

@chfast are you still pursuing this? Would need to be rebased/merged onto master first.

@chfast
Copy link
Member Author

chfast commented Oct 16, 2018

It's quite outdated and not worth to keep it around.

@chfast chfast closed this Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pirapira pirapira pirapira left review comments

@Arachnid Arachnid Arachnid left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chfast @pirapira @Arachnid @axic