Skip to content

How to write tests for the consensus layer#2700

Merged
djrtwo merged 47 commits intoethereum:devfrom
qbzzt:dev
Nov 22, 2021
Merged

How to write tests for the consensus layer#2700
djrtwo merged 47 commits intoethereum:devfrom
qbzzt:dev

Conversation

@qbzzt
Copy link
Contributor

@qbzzt qbzzt commented Oct 30, 2021

No description provided.

Copy link
Contributor

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

Hi Ori, nice to find you here in the consensus specs repo, thanks for the help documenting testing!

Some nitpicks in review, but generally this is very helpful, thank you 👍

@qbzzt
Copy link
Contributor Author

qbzzt commented Nov 1, 2021

Hi Ori, nice to find you here in the consensus specs repo, thanks for the help documenting testing!

Some nitpicks in review, but generally this is very helpful, thank you 👍

Nice to find you here too. Yes, I've been writing helping out EF with documentation and testing for a while now, long before I've even heard of Optimism.

I'm glad it helps. It's always difficult to know how much detail to put in a tutorial.

@qbzzt qbzzt marked this pull request as draft November 1, 2021 02:21
@qbzzt qbzzt requested a review from protolambda November 6, 2021 01:57
@qbzzt qbzzt marked this pull request as ready for review November 20, 2021 13:05
Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

nice! just a handful of review comments


This type of test receives two parameters:

* `specs`: The protocol specifications
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* `specs`: The protocol specifications
* `spec`: The protocol specification (constants, functions, object definitions, etc)

tests/README.md Outdated

For every slot a validator is randomly selected as the proposer. Currently the proposer proposes the hash
for the current head of the beacon chain (the previous block). When shards are added, the proposer will also
propose a hash for the head of the assigned shard.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
propose a hash for the head of the assigned shard.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just cut out a lot of the sharding stuff. There are actually separate proposrs that build the shard chains and it's probably not worth getting in the nuance here.

qbzzt and others added 7 commits November 22, 2021 11:27
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
@qbzzt qbzzt requested a review from djrtwo November 22, 2021 17:38
@djrtwo djrtwo merged commit 9999331 into ethereum:dev Nov 22, 2021
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