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

Executable Merkle proofs #1363

Merged
merged 13 commits into from Aug 23, 2019
Merged

Executable Merkle proofs #1363

merged 13 commits into from Aug 23, 2019

Conversation

hwwhww
Copy link
Contributor

@hwwhww hwwhww commented Aug 15, 2019

Issue

#1323 needs some shared logic in specs/light_client/merkle_proofs.md. And also, we can use the helper functions in merkle_proofs.md to generate the test cases for #1323.

Description

This PR is making merkle_proofs.md executable.

TODOs:

  • Fix all linter errors
  • Add basic tests

specs/light_client/merkle_proofs.md Show resolved Hide resolved
specs/light_client/merkle_proofs.md Outdated Show resolved Hide resolved
We define the following Python custom types for type hinting and readability:

| - | - | - |
| `GeneralizedIndex` | `uint64` | the index of a node in a binary Merkle tree |
Copy link
Contributor

Choose a reason for hiding this comment

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

A GeneralizedIndex is NOT necessarily a uint64. It could be bigger. My idea was to represent it in the python executable as a bigint, but then clarify that representing it as a bitfield/bitlist is ok too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in bbaa238.

Copy link
Collaborator

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

LGTM

@protolambda protolambda merged commit 5d2f34f into dev Aug 23, 2019
@protolambda protolambda deleted the executable_merkle_proofs branch August 23, 2019 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants