[Merged by Bors] - Improve ergonomics of adding a new network config by paulhauner · Pull Request #2489 · sigp/lighthouse

paulhauner · 2021-08-02T06:06:39Z

Issue Addressed

NA

Proposed Changes

This PR adds some more fancy macro magic to make it easier to add a new built-in (aka "baked-in") testnet config to the lighthouse binary.

Previously, a user needed to modify several files and repeat themselves several times. Now, they only need to add a single definition in the eth2_config crate. No repetition 🎉

commit e22b271 Author: Paul Hauner <paul@paulhauner.com> Date: Wed Aug 4 10:55:15 2021 +1000 Separate name_ident and name_str commit d7e8408 Author: Paul Hauner <paul@paulhauner.com> Date: Tue Aug 3 10:58:54 2021 +1000 Naming consistency fix commit cd73518 Author: Paul Hauner <paul@paulhauner.com> Date: Tue Aug 3 10:44:26 2021 +1000 Refactor for a single source of truth commit 83280d6 Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 18:27:51 2021 +1000 Add test for name consistency commit 99be2ea Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 15:55:40 2021 +1000 Switch define_nets to lower case commit 777b419 Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 15:52:41 2021 +1000 Add instructional comments commit 33dda79 Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 15:36:14 2021 +1000 Use hardcoded list in CLI commit 892efe4 Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 15:34:58 2021 +1000 Add paste to eth2_config mod commit 03d6e49 Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 15:23:06 2021 +1000 Add hardcoded net names commit d3dadc8 Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 15:10:01 2021 +1000 Use paste in eth2_network_config commit 5053f53 Author: Paul Hauner <paul@paulhauner.com> Date: Mon Aug 2 15:03:15 2021 +1000 Add define_nets!

realbigsean

This looks sweet! Quite the macro rabbit hole here, didn't realize how much one can do. Just found some small things.

realbigsean · 2021-08-10T19:58:28Z

common/eth2_config/src/lib.rs

+/// perform the final step of using `std::include_bytes` to bake the files (bytes) into the binary.
+macro_rules! define_hardcoded_nets {
+    ($(($name_ident: ident, $name_str: tt, $genesis_is_known: ident)),+) => {
+        paste! {


I don't see any paste! identifiers [< .. >] in the macro so not sure it's necessary here

realbigsean · 2021-08-10T20:16:22Z

common/eth2_config/src/lib.rs

+/// Creates a `HardcodedNet` definition for some network.
+#[macro_export]
+macro_rules! define_net {
+    ($this_crate: tt, $mod: ident, $include_file: tt) => {{


Suggested change

($this_crate: tt, $mod: ident, $include_file: tt) => {{

($this_crate: ident, $mod: ident, $include_file: tt) => {{

Seems like this works as is, but should this be ident?

realbigsean · 2021-08-10T20:22:08Z

common/eth2_config/src/lib.rs

+
+/// Defines an `Eth2NetArchiveAndDirectory` for some network.
+///
+/// It also defines a `include_<title>_bytes!` macro which provides a wrapper around


Suggested change

/// It also defines a `include_<title>_bytes!` macro which provides a wrapper around

/// It also defines a `include_<title>_file!` macro which provides a wrapper around

paulhauner · 2021-08-30T00:13:14Z

Thanks @realbigsean, great suggestions. Ready for review!

realbigsean

LGTM!

paulhauner · 2021-08-30T23:27:12Z

bors r+

## Issue Addressed NA ## Proposed Changes This PR adds some more fancy macro magic to make it easier to add a new built-in (aka "baked-in") testnet config to the `lighthouse` binary. Previously, a user needed to modify several files and repeat themselves several times. Now, they only need to add a single definition in the `eth2_config` crate. No repetition 🎉

bors · 2021-08-31T01:13:56Z

Pull request successfully merged into unstable.

Build succeeded:

paulhauner added 7 commits August 2, 2021 15:03

Add define_nets!

5053f53

Use paste in eth2_network_config

d3dadc8

Add hardcoded net names

03d6e49

Add paste to eth2_config mod

892efe4

Use hardcoded list in CLI

33dda79

Add instructional comments

777b419

Switch define_nets to lower case

99be2ea

paulhauner added work-in-progress PR is a work-in-progress v1.5.1 To be included in the v1.5.1 relase labels Aug 2, 2021

Add test for name consistency

83280d6

paulhauner added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Aug 2, 2021

paulhauner added 3 commits August 3, 2021 10:44

Refactor for a single source of truth

cd73518

Naming consistency fix

d7e8408

Separate name_ident and name_str

e22b271

paulhauner mentioned this pull request Aug 4, 2021

Altair devnets #2450

Closed

realbigsean requested changes Aug 10, 2021

View reviewed changes

realbigsean added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Aug 10, 2021

michaelsproul added v1.5.2 The release after v1.5.1 v2.0.0 Altair on mainnet release (v2.0.0) and removed v1.5.1 To be included in the v1.5.1 relase v1.5.2 The release after v1.5.1 labels Aug 26, 2021

Address review comments

b34ae67

paulhauner removed the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Aug 30, 2021

paulhauner added ready-for-review The code is ready for review v1.5.2 The release after v1.5.1 and removed v2.0.0 Altair on mainnet release (v2.0.0) labels Aug 30, 2021

realbigsean approved these changes Aug 30, 2021

View reviewed changes

paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 30, 2021

bors bot changed the title ~~Improve ergonomics of adding a new network config~~ [Merged by Bors] - Improve ergonomics of adding a new network config Aug 31, 2021

bors bot closed this Aug 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Improve ergonomics of adding a new network config#2489

[Merged by Bors] - Improve ergonomics of adding a new network config#2489
paulhauner wants to merge 12 commits intosigp:unstablefrom
paulhauner:testnet-macros

paulhauner commented Aug 2, 2021 •

edited

Loading

Uh oh!

realbigsean left a comment

Uh oh!

realbigsean Aug 10, 2021

Uh oh!

realbigsean Aug 10, 2021

Uh oh!

realbigsean Aug 10, 2021

Uh oh!

paulhauner commented Aug 30, 2021

Uh oh!

realbigsean left a comment

Uh oh!

paulhauner commented Aug 30, 2021

Uh oh!

bors bot commented Aug 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	($this_crate: tt, $mod: ident, $include_file: tt) => {{
	($this_crate: ident, $mod: ident, $include_file: tt) => {{

	/// It also defines a `include_<title>_bytes!` macro which provides a wrapper around
	/// It also defines a `include_<title>_file!` macro which provides a wrapper around

Conversation

paulhauner commented Aug 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue Addressed

Proposed Changes

Uh oh!

realbigsean left a comment

Choose a reason for hiding this comment

Uh oh!

realbigsean Aug 10, 2021

Choose a reason for hiding this comment

Uh oh!

realbigsean Aug 10, 2021

Choose a reason for hiding this comment

Uh oh!

realbigsean Aug 10, 2021

Choose a reason for hiding this comment

Uh oh!

paulhauner commented Aug 30, 2021

Uh oh!

realbigsean left a comment

Choose a reason for hiding this comment

Uh oh!

paulhauner commented Aug 30, 2021

Uh oh!

bors bot commented Aug 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paulhauner commented Aug 2, 2021 •

edited

Loading