Skip to content

Conversation

@narimiran
Copy link
Contributor

Also fixes some existing outdated/broken links.

Also fixes some existing outdated/broken links.
In the `nimbus-eth1` directory, run the following commands:
```
git pull
make -j4 update
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it best specifically to use -j4, is this a compromise for systems where make -j$(nproc) update might fail because they don't have nproc as such, etc?

Also, I'm not familiar with how nimbus-eth1 currently builds, but it seems likely that this doesn't build nimbus-eth1 but rather updates the submodules?

I guess this whole section borrows from https://github.com/status-im/nimbus-eth1#posix-compatible-os but it looks like the actual build command is make nimbus (make -j$foo V=$bar nimbus, etc), and git pull && make -j4 update ("Assuming you have 4 CPU cores available") just sets up the local git clone for that make nimbus.

But I might be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess this whole section borrows from https://github.com/status-im/nimbus-eth1#posix-compatible-os

Correct, it was taken from there.

it seems likely that this doesn't build nimbus-eth1 but rather updates the submodules?

If these commands are incorrect/incomplete, then, IMO, nimbus-eth1's readme should be more specific and/or write all the commands needed. Currently it says:

# Update to latest version
git pull
make -j4 update

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, agree, the documentation here accurately reflects what the nimbus-eth1 README.md states, and if that's unclear or incomplete, that's unclear or incomplete there too.

Copy link
Contributor

Choose a reason for hiding this comment

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

I did a quick update to the nimbus-eth1 readme to reflect the update process (its the same as nimbus-eth2 basically): status-im/nimbus-eth1#1943

I do question if we should put nimbus EL here in the list like the other EL clients.


!!! info
When the `--jwt-secret` option is not specified and the execution client is running on the same machine under default setting, Nimbus may be able to connect successfully to it by using the default secret value `0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3`.
This is a [proposed standard protocol](https://github.com/ethereum/execution-apis/pull/302) that aims to simplify the required user configuration, but it's not yet adopted by all execution clients.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this isn't added or changed by this PR, but I'm not sure why one would note this in documentation about how to use a client which exists today. For the purposes of people using Nimbus right now or in the near future, it seems irrelevant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, the whole info block (lines 97-99) should be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, the first two lines (97 and 98):

!!! info
When the --jwt-secret option is not specified and the execution client is running on the same machine under default setting, Nimbus may be able to connect successfully to it by using the default secret value 0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3.

Probably, though I want to flag that @arnetheduck might have views on this as a stakeholder.

That default JWT secret value is, yes, in nimbus-eth2. It's from ethereum/execution-apis#302 so to the extent this PR is or isn't implemented (there doesn't seem to have been a lot of activity on it in the last few months), and from the participants, I might guess that Lodestar maybe/probably has this, but not sure if anyone else.

So there's not some longstanding convention of doing this; lines 97 and 98 are just restating line 99 more descriptively.

Line 99:

This is a [proposed standard protocol](https://github.com/ethereum/execution-apis/pull/302) that aims to simplify the required user configuration, but it's not yet adopted by all execution clients.

I would suggest should be removed, yes, because it's not that useful for this guide to be ... aspirational in this way. If it were a widely-agreed-upon convention that just by chance hadn't made it into the engine API specs, sure. But that is not the case here. The guide ideally describes what is, not what one might think should be.

And then, because the specific value is just a restatement of this, yeah, I'd remove all three lines.

Copy link
Contributor

Choose a reason for hiding this comment

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

And as a pure UX aspect of the guide, one concern is that it has such a low probability of working it's a net cost rather than asset to include -- it creates (usually) false expectations, it can wastes people's time when they try this thing which is, again, aspirational rather than real and deployed on the EL end, and as a result risks interfering with actual troubleshooting steps, or worse, making people think that they're done without having set up JWT.

Whatever one's preferences for how JWT should work, right now, it's a mandatory, required part of CL/EL configuration. Suggesting otherwise is misleading.

Copy link
Contributor

Choose a reason for hiding this comment

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

Checked five current ELs:

$ git clone --depth 1 --recurse-submodules --shallow-submodules --branch v2.55.1 https://github.com/ledgerwatch/erigon && git clone --depth 1 --recurse-submodules --shallow-submodules --branch v0.1.0-alpha.13 https://github.com/paradigmxyz/reth && git clone --depth 1 --recurse-submodules --shallow-submodules --branch 1.23.0 https://github.com/NethermindEth/nethermind && git clone --depth 1 --recurse-submodules --shallow-submodules --branch v1.13.5 https://github.com/ethereum/go-ethereum && git clone --depth 1 --recurse-submodules --shallow-submodules --branch 23.10.2 https://github.com/hyperledger/besu && echo got all EL client source
...
got all EL client source
$ grep -rin d4e5674
erigon/tests/testdata/BasicTests/genesishashestest.json:4:  "genesis_hash": "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"
erigon/cmd/rpcdaemon/postman/RPC_Testing.json:867:                      "    \"parentHash\": \"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3\",",
erigon/cmd/rpcdaemon/postman/RPC_Testing.json:946:                      "    \"parentHash\": \"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3\",",
erigon/cmd/rpcdaemon/postman/RPC_Testing.json:2721:                  "    \"genesis\": \"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3\",",
erigon/params/config.go:53:	MainnetGenesisHash    = libcommon.HexToHash("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3")
besu/ethereum/referencetests/src/reference-test/external-resources/BasicTests/genesishashestest.json:4:  "genesis_hash": "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"
besu/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/forkid/ForkIdTestUtil.java:64:        "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3";
go-ethereum/tests/testdata/BasicTests/genesishashestest.json:4:  "genesis_hash": "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"
go-ethereum/params/config.go:28:	MainnetGenesisHash = common.HexToHash("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3")
reth/crates/primitives/src/constants/mod.rs:109:    b256!("d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3");
reth/crates/primitives/src/chain/spec.rs:26:            "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"
reth/crates/primitives/src/block.rs:483:            B256::from_str("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3")
reth/crates/primitives/src/block.rs:486:            { "blockHash": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"}
reth/crates/primitives/src/block.rs:494:            B256::from_str("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3")
reth/crates/primitives/src/block.rs:498:            { "blockHash": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3", "requireCanonical": true }
reth/crates/primitives/src/block.rs:566:        let example_payload = r#"{"method":"eth_call","params":[{"to":"0xebe8efa441b9302a0d7eaecc277c09d20d684540","data":"0x45848dfc"},{"blockHash": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"}],"id":1,"jsonrpc":"2.0"}"#;
reth/crates/primitives/src/block.rs:571:            B256::from_str("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3")
reth/crates/primitives/src/block.rs:575:        assert_eq!("{\"blockHash\":\"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3\"}", serialized)
reth/crates/primitives/src/block.rs:596:        let payload = r#"{"blockHash": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"}"#;
reth/crates/primitives/src/block.rs:599:            B256::from_str("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3")
reth/crates/ethereum-forks/src/forkid.rs:386:        b256!("d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3");
reth/crates/net/eth-wire/src/types/status.rs:160:        let expected = hex!("f85643018a07aac59dabcdd74bc567a0feb27336ca7923f8fab3bd617fcb6e75841538f71c1bcfc267d7838489d9e13da0d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3c684b715077d80");
reth/crates/net/eth-wire/src/types/status.rs:170:                "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3",
reth/crates/net/eth-wire/src/types/status.rs:183:        let data = hex!("f85643018a07aac59dabcdd74bc567a0feb27336ca7923f8fab3bd617fcb6e75841538f71c1bcfc267d7838489d9e13da0d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3c684b715077d80");
reth/crates/net/eth-wire/src/types/status.rs:193:                "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3",
reth/crates/rpc/rpc-types/src/admin.rs:98:        let sample = r#"{"enode":"enode://44826a5d6a55f88a18298bca4773fca5749cdc3a5c9f308aa7d810e9b31123f3e7c5fba0b1d70aac5308426f47df2a128a6747040a3815cc7dd7167d03be320d@[::]:30303","id":"44826a5d6a55f88a18298bca4773fca5749cdc3a5c9f308aa7d810e9b31123f3e7c5fba0b1d70aac5308426f47df2a128a6747040a3815cc7dd7167d03be320d","ip":"::","listenAddr":"[::]:30303","name":"reth","ports":{"discovery":30303,"listener":30303},"protocols":{"eth":{"difficulty":17334254859343145000,"genesis":"0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3","head":"0xb83f73fbe6220c111136aefd27b160bf4a34085c65ba89f24246b3162257c36a","network":1}}}"#;
reth/book/jsonrpc/admin.md:105:                "genesis": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3",
nethermind/src/Nethermind/Nethermind.Specs/KnownHashes.cs:10:        public static readonly Hash256 MainnetGenesis = new("0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3");
nethermind/src/Nethermind/Nethermind.JsonRpc/Modules/Admin/NodeInfo.cs:23:    ///         genesis: "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3",
nethermind/src/Nethermind/Nethermind.Runner.Test/ConfigFilesTests.cs:98:        [TestCase("mainnet", "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3")]
nethermind/src/Nethermind/Nethermind.Runner/configs/mainnet_archive.cfg:4:    "GenesisHash": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3",
nethermind/src/Nethermind/Nethermind.Runner/configs/mainnet.cfg:4:    "GenesisHash": "0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3",
nethermind/src/tests/BasicTests/genesishashestest.json:4:  "genesis_hash": "d4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3"

Every one of these appears to one or both of testing and referring to the genesis hash role of 0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3. I see no evidence that the verbiage in the Nimbus guide of

Nimbus may be able to connect successfully to it by using the default secret value 0xd4e56740f876aef8c010b86a40d5f56745a118d0906a34e69aec8c0db1cb8fa3

is meaningfully true or useful. That is, if one configures an EL with that JWT secret, of course it can work, but that's not the spirit of the documentation as written.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, this came in from 17c0eee#diff-3a5a19ec4d04c2a998dfd8d21bc608f27dc23f6504863bfab55f7209f951fb0e

I'd say safe to revert, unless @zah wants to provide further context.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, this note describes the precise behavior of Nimbus, so I don't think it's completely useless. I agree that due to lack of adoption from other clients, it's unlikely that users are actually taking advantage of it, but it does explain why Nimbus doesn't have an error message for a missing JWT value in the configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, precisely, Nimbus will attempt to use that default value, but for it to be successful, the EL has to support it too.

As far as I can tell, exactly none of the ELs actually do this. Nimbus can try, but unless explicitly configured on the EL side (at which point, the default status isn't as useful), it will not be successful.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's not basically unused because users are just "not taking advantage of it", it's because it doesn't work if people try. It's a waste of time. There's nothing to take advantage of unless at least one EL implements this.

@tersec
Copy link
Contributor

tersec commented Dec 4, 2023

Worth including https://paradigmxyz.github.io/reth/installation/installation.html yet? They're still not 100% released, so I can see arguments either way.

@mratsim
Copy link
Contributor

mratsim commented Dec 10, 2023

reth is still in beta and not fuzzed and audited as far as I know.

The revm itself wasn't fuzzed yet from my talk with Dragan at Devconnect. So it needs some beta or preview indication.

@zah zah merged commit f125a5c into unstable Dec 16, 2023
@zah zah deleted the fix-2176 branch December 16, 2023 16:34
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.

5 participants