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

Gather validator with beacon node plugin #594

Merged
merged 11 commits into from
May 16, 2019

Conversation

mhchia
Copy link
Contributor

@mhchia mhchia commented May 9, 2019

What was wrong?

Merge the rest of #465 back.

How was it fixed?

Based on #587.

  • Parse genesis state and block, and privkeys from pre-generated files from basic trinity-beacon testnet command #517.
  • Spin up Validator, SlotTicker in beacon node plugin based on the genesis files and privkeys.
  • Add script run_beacon_node.py and tests for trinity-beacon.

To-Do

  • Fix tests/plugins/eth2/beacon/test_beacon_node_syncing.py.
  • Fix validator test, since the API changed.
  • Clean up for unnecessary code, mainly for the tests.

Cute Animal Picture

put a cute animal picture link inside the parentheses

@mhchia
Copy link
Contributor Author

mhchia commented May 9, 2019

Sorry, I forgot there are still todos undone and added reviewers. Please ignore this PR for now and review it later after the todos are solved.

@mhchia mhchia force-pushed the feature/validator-in-beacon-node-plugin branch from 99b3ba7 to f294f76 Compare May 13, 2019 09:42
num_validators = 5
cmd_gen_testnet_files = f"trinity-beacon testnet --num={num_validators} --network-dir={dir_root}"
cmd_alice = f"trinity-beacon --port={port_alice} --trinity-root-dir={dir_alice} --beacon-nodekey=6b94ffa2d9b8ee85afb9d7153c463ea22789d3bbc5d961cc4f63a41676883c19 -l debug" # noqa: E501
cmd_bob = f"trinity-beacon --port={port_bob} --trinity-root-dir={dir_bob} --beacon-nodekey=f5ad1c57b5a489fc8f21ad0e5a19c1f1a60b8ab357a2100ff7e75f3fa8a4fd2e --bootstrap_nodes=enode://c289557985d885a3f13830a475d649df434099066fbdc840aafac23144f6ecb70d7cc16c186467f273ad7b29707aa15e6a50ec3fde35ae2e69b07b3ddc7a36c7@127.0.0.1:{port_alice} -l debug" # noqa: E501
Copy link
Contributor

Choose a reason for hiding this comment

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

These nodekeys and enode address can be extracted as new variables

eth2/beacon/scripts/run_beacon_nodes.py Show resolved Hide resolved
f"rm -rf {dir_root}"
)
await proc.wait()
# proc = await run(
Copy link
Contributor

Choose a reason for hiding this comment

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

remove comment

await asyncio.sleep(time_bob_wait_for_alice)

node_bob = Node('Bob\t0e01b', cmd_bob)
# node_bob.add_event("stderr", START_SYNCING)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unused comment

@mhchia mhchia force-pushed the feature/validator-in-beacon-node-plugin branch 2 times, most recently from 9a74dbc to 831e761 Compare May 14, 2019 08:30
@mhchia mhchia changed the title [WIP] Gather validator with beacon node plugin Gather validator with beacon node plugin May 14, 2019
@mhchia
Copy link
Contributor Author

mhchia commented May 14, 2019

@ChihChengLiang @NIC619 This should be ready for the review:)

@mhchia mhchia force-pushed the feature/validator-in-beacon-node-plugin branch from c647f63 to bb1b295 Compare May 14, 2019 10:57
tests/plugins/eth2/beacon/test_validator.py Outdated Show resolved Hide resolved
@@ -239,23 +251,29 @@ def _get_slot_with_validator_selected(largest_index, start_slot, state, state_ma


@pytest.mark.asyncio
async def test_validator_new_slot(caplog, event_loop, event_bus, monkeypatch):
caplog.set_level(logging.DEBUG)
async def test_validator_propose_or_skip_block_propose(event_loop, event_bus, monkeypatch):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we merge this and test_validator_propose_or_skip_block_skip into one test? The names are quite confusing and the codes are mostly the same.


def extract_genesis_state_from_stream(stream: Union[Path, "StreamTextType"]) -> BeaconState:
yaml = YAML(typ="unsafe")
genesis_json = yaml.load(stream)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to check if the genesis file is indeed there? Or the from_formatted_dict will raise error if it's empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yaml.load raises ruamel.yaml.scanner.ScannerError if the content is malformed. IMO it's fine to let the exception raise here.

trinity/plugins/eth2/beacon/utils.py Show resolved Hide resolved
trinity/plugins/eth2/beacon/utils.py Show resolved Hide resolved
trinity/config.py Outdated Show resolved Hide resolved
trinity/plugins/eth2/beacon/plugin.py Outdated Show resolved Hide resolved

slot_ticker = SlotTicker(
genesis_slot=state_machine.config.GENESIS_SLOT,
genesis_time=state.genesis_time,
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems weird to access GENESIS_TIME from state(though it IS part of the state). IMO it's more proper to get this from a chain_config or chain. cc @hwwhww @ChihChengLiang

Copy link
Contributor Author

@mhchia mhchia May 15, 2019

Choose a reason for hiding this comment

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

I added a field genesis_time in genesis_data, whereas it is still initialized from state.genesis_time.

tests/plugins/eth2/beacon/helpers.py Outdated Show resolved Hide resolved
eth2/beacon/scripts/run_beacon_nodes.py Outdated Show resolved Hide resolved
trinity/plugins/eth2/beacon/utils.py Outdated Show resolved Hide resolved
mhchia added 11 commits May 16, 2019 12:55
Fix the test for `_try_import_orphan_blocks`

TODO: add tests for the rest of new methods

Add tests

For `_process_received_block`, `_broadcast_block`, and `import_block`.

Add comments to BCCReceiveServer

Add more comments and modify some.

Add beacon plugin and SyncBlockImporter from ethereum#465

Move files from ethereum#465

Add utils to work with testnet

And modify original code to accommodate with the new desing

Minor fix

Modify `BeaconChainSyncer`

Plugin works with `run_beacon_nodes.py` now!

Clean up for linters
- Fix `test_validator.py`
- Remove `test_beacon_node_syncing.py`
- Add a test for `propose_or_skip_block`
- Remove left-over merge directives
- Reduce hardcoded values
- Move globals to locals or static class var
- Remove useless comment and print
- Add running nodes to `Node.running_nodes`
- Rename `NopBlockImporter` to `NoopBlockImporter`
- Add `genesis_time` in `BeaconGenesisData`
- Merge tests of `propose_or_skip_block`
- Sort import statements
- Add FIXME for using `state_machine_class.config`
- Remove unneeded `loop.run_forever()`
- Add `PrivateKeyNotFound` when parsing validator's private keys
- Rename `_extract_privkey_from_stream` to `_read_privkey`
@mhchia mhchia force-pushed the feature/validator-in-beacon-node-plugin branch from 771abc3 to af9cf66 Compare May 16, 2019 04:55
@mhchia mhchia merged commit 15958c3 into ethereum:master May 16, 2019
@mhchia mhchia deleted the feature/validator-in-beacon-node-plugin branch May 16, 2019 05:25
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.

3 participants