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.

Sign up for GitHub

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

Update nim-eth types #6583

Merged
merged 7 commits into from
Sep 29, 2024
Merged

Update nim-eth types #6583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aa1464b
Update nim-eth types
arnetheduck Sep 26, 2024
ad24d44
fix ncli
arnetheduck Sep 26, 2024
345a76c
better workaround for rlp encode issue
arnetheduck Sep 26, 2024
3299011
comment
arnetheduck Sep 26, 2024
6a6a375
bump
arnetheduck Sep 26, 2024
425df59
bump
arnetheduck Sep 28, 2024
c6f5044
bump
arnetheduck Sep 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions beacon_chain/conf.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,8 @@ type

AnyConf* = BeaconNodeConf | ValidatorClientConf | SigningNodeConf

Address = primitives.Address

Comment on lines +1136 to +1137
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is not exported, and it worked before without it, is it necessary?

Copy link
Member Author

@arnetheduck arnetheduck Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

nim-eth adds an Address symbol - this local type has precedence over everything imported so it sets a preference between primitives and nim-eth for the rest of the file

proc defaultDataDir*[Conf](config: Conf): string =
let dataDir = when defined(windows):
"AppData" / "Roaming" / "Nimbus"
Expand Down
2 changes: 2 additions & 0 deletions beacon_chain/el/el_manager.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ logScope:
topics = "elman"

type
FixedBytes[N: static int] = web3.FixedBytes[N]
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
FixedBytes[N: static int] = web3.FixedBytes[N]
FixedBytes[N: static int] = web3.FixedBytes[N]

this one is also not exported and it worked without this definition before.

PubKeyBytes = DynamicBytes[48, 48]
WithdrawalCredentialsBytes = DynamicBytes[32, 32]
SignatureBytes = DynamicBytes[96, 96]
Int64LeBytes = DynamicBytes[8, 8]
WithoutTimeout* = distinct int
Address = web3.Address

SomeEnginePayloadWithValue =
BellatrixExecutionPayloadWithValue |
Expand Down
106 changes: 52 additions & 54 deletions beacon_chain/libnimbus_lc/libnimbus_lc.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1282,25 +1282,25 @@ proc ETHExecutionBlockHeaderCreateFromJson(
if data.nonce.isNone:
return nil
let blockHeader = ExecutionBlockHeader(
parentHash: data.parentHash.asEth2Digest,
ommersHash: data.sha3Uncles.asEth2Digest,
coinbase: distinctBase(data.miner),
stateRoot: data.stateRoot.asEth2Digest,
txRoot: data.transactionsRoot.asEth2Digest,
receiptsRoot: data.receiptsRoot.asEth2Digest,
logsBloom: distinctBase(data.logsBloom),
parentHash: data.parentHash.asEth2Digest.to(Hash32),
ommersHash: data.sha3Uncles.asEth2Digest.to(Hash32),
coinbase: distinctBase(data.miner).to(EthAddress),
stateRoot: data.stateRoot.asEth2Digest.to(Hash32),
transactionsRoot: data.transactionsRoot.asEth2Digest.to(Hash32),
receiptsRoot: data.receiptsRoot.asEth2Digest.to(Hash32),
logsBloom: distinctBase(data.logsBloom).to(Bloom),
difficulty: data.difficulty,
number: distinctBase(data.number),
gasLimit: distinctBase(data.gasLimit),
gasUsed: distinctBase(data.gasUsed),
timestamp: EthTime(distinctBase(data.timestamp)),
extraData: distinctBase(data.extraData),
mixHash: data.mixHash.asEth2Digest,
nonce: distinctBase(data.nonce.get),
mixHash: data.mixHash.asEth2Digest.to(Hash32),
nonce: distinctBase(data.nonce.get).to(Bytes8),
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't nonce be a number type? is it still getting serialized properly with this change? as in, as Quantity and not as a byte array, on engine?

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/ethereum/execution-specs/blob/51fac24740e662844446439ceeb96a460aae0ba0/src/ethereum/paris/blocks.py#L41

Copy link
Contributor

Choose a reason for hiding this comment

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

is that compatible with engine API?

Copy link
Contributor

Choose a reason for hiding this comment

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

(or, json-rpc, probably the better one, as nonce is not used in engine itself)

Copy link
Contributor

Choose a reason for hiding this comment

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

https://ethereum.org/en/developers/docs/apis/json-rpc/#eth_getblockbyhash says it's bytes8, but the reality seems to encode it as quantity:

 curl https://docs-demo.quiknode.pro/ \
  -X POST \
  -H "Content-Type: application/json" \
  --data '{"method":"eth_getBlockByNumber","params":["0xc5043f",true],"id":1,"jsonrpc":"2.0"}' | jq '.'

e.g. here nonce is "0x1d" not a bytes8

      {
        "blockHash": "0xa917fcc721a5465a484e9be17cda0cc5493933dd3bc70c9adbee192cb419c9d7",
        "blockNumber": "0xc5043f",
        "from": "0xb0cba44805f096094e8759ee9824a05590c10f27",
        "gas": "0x5208",
        "gasPrice": "0x7f084b500",
        "hash": "0x3dc2776aa483c0eee09c2ccc654bf81dccebead40e9bb664289637bfb5e7e954",
        "input": "0x",
        "nonce": "0x1d",
        "to": "0xb0cba44805f096094e8759ee9824a05590c10f27",
        "transactionIndex": "0xcb",
        "value": "0x3f3b45b9331",
        "type": "0x0",
        "chainId": "0x1",
        "v": "0x26",
        "r": "0x19610db3a46538e10c4db8597510902374ff66ef339a2251c82faebb3c00f8b9",
        "s": "0x7af4c008f4bc1edbe081c8e32f49893f035a9eeacb615dd67808f6a9f5e13b52"
      }

the specs don't seem authoritative. https://github.com/ethereum/devp2p/blob/master/caps/eth.md here the blocks don't even have excess blob fees or parent beacn blob roots, as per spec, in reality they have.

or also, there's ethereum/execution-apis#573 which is implemented in practice but is not specced out like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

BytesX generally also encodes as 0x... in the new nim-eth - but that doesn't matter, we're dealing with an eth type here, not web3 - this is internal to the execution client, not a web3 impl

Copy link
Contributor

@jangko jangko Sep 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ExecutionPayload of web3 is not using nonce from eth block header. And the example posted by @etan-status is not a block header, but a transaction. And a transaction's nonce is a uint64. There will be no conversion friction or serialization issue in the case of this block header nonce. And there is no problem also with the json encoding nonce of block header so far in our json-rpc.

baseFeePerGas: data.baseFeePerGas,
withdrawalsRoot:
if data.withdrawalsRoot.isSome:
Opt.some(data.withdrawalsRoot.get.asEth2Digest)
Opt.some(data.withdrawalsRoot.get.asEth2Digest.to(Hash32))
else:
Opt.none(ExecutionHash256),
blobGasUsed:
Expand All @@ -1315,12 +1315,12 @@ proc ETHExecutionBlockHeaderCreateFromJson(
Opt.none(uint64),
parentBeaconBlockRoot:
if data.parentBeaconBlockRoot.isSome:
Opt.some distinctBase(data.parentBeaconBlockRoot.get.asEth2Digest)
Opt.some data.parentBeaconBlockRoot.get.asEth2Digest.to(Hash32)
else:
Opt.none(ExecutionHash256),
requestsRoot:
if data.requestsRoot.isSome:
Opt.some(data.requestsRoot.get.asEth2Digest)
Opt.some(data.requestsRoot.get.asEth2Digest.to(Hash32))
else:
Opt.none(ExecutionHash256))
if rlpHash(blockHeader) != executionHash[]:
Expand All @@ -1342,7 +1342,7 @@ proc ETHExecutionBlockHeaderCreateFromJson(
wd = ExecutionWithdrawal(
index: distinctBase(data.index),
validatorIndex: distinctBase(data.validatorIndex),
address: distinctBase(data.address),
address: distinctBase(data.address).to(EthAddress),
amount: distinctBase(data.amount))
rlpBytes =
try:
Expand All @@ -1353,7 +1353,7 @@ proc ETHExecutionBlockHeaderCreateFromJson(
wds.add ETHWithdrawal(
index: wd.index,
validatorIndex: wd.validatorIndex,
address: ExecutionAddress(data: wd.address),
address: ExecutionAddress(data: wd.address.data),
amount: wd.amount,
bytes: rlpBytes)

Expand All @@ -1379,10 +1379,10 @@ proc ETHExecutionBlockHeaderCreateFromJson(
# Construct deposit request
let
req = ExecutionDepositRequest(
pubkey: distinctBase(data.pubkey),
withdrawalCredentials: distinctBase(data.withdrawalCredentials),
pubkey: distinctBase(data.pubkey).to(Bytes48),
withdrawalCredentials: distinctBase(data.withdrawalCredentials).to(Bytes32),
amount: distinctBase(data.amount),
signature: distinctBase(data.signature),
signature: distinctBase(data.signature).to(Bytes96),
index: distinctBase(data.index))
rlpBytes =
try:
Expand All @@ -1391,10 +1391,10 @@ proc ETHExecutionBlockHeaderCreateFromJson(
raiseAssert "Unreachable"

depositRequests.add ETHDepositRequest(
pubkey: ValidatorPubKey(blob: req.pubkey),
withdrawalCredentials: req.withdrawalCredentials,
pubkey: ValidatorPubKey(blob: req.pubkey.data),
withdrawalCredentials: req.withdrawalCredentials.data,
amount: req.amount,
signature: ValidatorSig(blob: req.signature),
signature: ValidatorSig(blob: req.signature.data),
index: req.index,
bytes: rlpBytes)

Expand All @@ -1411,8 +1411,8 @@ proc ETHExecutionBlockHeaderCreateFromJson(
# Construct withdrawal request
let
req = ExecutionWithdrawalRequest(
sourceAddress: distinctBase(data.sourceAddress),
validatorPubkey: distinctBase(data.validatorPubkey),
sourceAddress: distinctBase(data.sourceAddress).to(EthAddress),
validatorPubkey: distinctBase(data.validatorPubkey).to(Bytes48),
amount: distinctBase(data.amount))
rlpBytes =
try:
Expand All @@ -1421,8 +1421,8 @@ proc ETHExecutionBlockHeaderCreateFromJson(
raiseAssert "Unreachable"

withdrawalRequests.add ETHWithdrawalRequest(
sourceAddress: ExecutionAddress(data: req.sourceAddress),
validatorPubkey: ValidatorPubKey(blob: req.validatorPubkey),
sourceAddress: ExecutionAddress(data: req.sourceAddress.data),
validatorPubkey: ValidatorPubKey(blob: req.validatorPubkey.data),
amount: req.amount,
bytes: rlpBytes)

Expand All @@ -1439,19 +1439,19 @@ proc ETHExecutionBlockHeaderCreateFromJson(
# Construct consolidation request
let
req = ExecutionConsolidationRequest(
sourceAddress: distinctBase(data.sourceAddress),
sourcePubkey: distinctBase(data.sourcePubkey),
targetPubkey: distinctBase(data.targetPubkey))
sourceAddress: distinctBase(data.sourceAddress).to(EthAddress),
sourcePubkey: distinctBase(data.sourcePubkey).to(Bytes48),
targetPubkey: distinctBase(data.targetPubkey).to(Bytes48))
rlpBytes =
try:
rlp.encode(req)
except RlpError:
raiseAssert "Unreachable"

consolidationRequests.add ETHConsolidationRequest(
sourceAddress: ExecutionAddress(data: req.sourceAddress),
sourcePubkey: ValidatorPubKey(blob: req.sourcePubkey),
targetPubkey: ValidatorPubKey(blob: req.targetPubkey),
sourceAddress: ExecutionAddress(data: req.sourceAddress.data),
sourcePubkey: ValidatorPubKey(blob: req.sourcePubkey.data),
targetPubkey: ValidatorPubKey(blob: req.targetPubkey.data),
bytes: rlpBytes)

# Verify requests root
Expand Down Expand Up @@ -1487,9 +1487,9 @@ proc ETHExecutionBlockHeaderCreateFromJson(
let executionBlockHeader = ETHExecutionBlockHeader.new()
executionBlockHeader[] = ETHExecutionBlockHeader(
transactionsRoot: blockHeader.txRoot,
withdrawalsRoot: blockHeader.withdrawalsRoot.get(ZERO_HASH),
withdrawalsRoot: blockHeader.withdrawalsRoot.get(zeroHash32),
withdrawals: wds,
requestsRoot: blockHeader.requestsRoot.get(ZERO_HASH),
requestsRoot: blockHeader.requestsRoot.get(zeroHash32),
depositRequests: depositRequests,
withdrawalRequests: withdrawalRequests,
consolidationRequests: consolidationRequests)
Expand Down Expand Up @@ -1776,31 +1776,31 @@ proc ETHTransactionsCreateFromJson(
gasLimit: distinctBase(data.gas).GasInt,
to:
if data.to.isSome:
Opt.some(distinctBase(data.to.get))
Opt.some(distinctBase(data.to.get).to(EthAddress))
else:
Opt.none(EthAddress),
value: data.value,
payload: data.input,
accessList:
if data.accessList.isSome:
data.accessList.get.mapIt(AccessPair(
address: distinctBase(it.address),
storageKeys: it.storageKeys.mapIt(distinctBase(it))))
address: distinctBase(it.address).to(EthAddress),
storageKeys: it.storageKeys.mapIt(distinctBase(it).to(Bytes32))))
else:
@[],
maxFeePerBlobGas:
data.maxFeePerBlobGas.get(0.u256),
versionedHashes:
if data.blobVersionedHashes.isSome:
data.blobVersionedHashes.get.mapIt(
ExecutionHash256(data: distinctBase(it)))
Bytes32(distinctBase(it)))
else:
@[],
authorizationList:
if data.authorizationList.isSome:
data.authorizationList.get.mapIt(Authorization(
chainId: it.chainId.ChainId,
address: distinctBase(it.address),
address: distinctBase(it.address).to(EthAddress),
nonce: distinctBase(it.nonce),
yParity: distinctBase(it.yParity),
R: it.R,
Expand Down Expand Up @@ -1834,7 +1834,7 @@ proc ETHTransactionsCreateFromJson(
return Opt.none(array[20, byte])
pubkey = sig.recover(sigHash).valueOr:
return Opt.none(array[20, byte])
Opt.some keys.PublicKey(pubkey).toCanonicalAddress()
Opt.some keys.PublicKey(pubkey).toCanonicalAddress().data

# Compute from execution address
let
Expand All @@ -1861,10 +1861,8 @@ proc ETHTransactionsCreateFromJson(
of DestinationType.Regular:
tx.to.get
of DestinationType.Create:
var res {.noinit.}: array[20, byte]
res[0 ..< 20] = keccakHash(rlp.encodeList(fromAddress, tx.nonce))
.data.toOpenArray(12, 31)
res
let hash = keccakHash(rlp.encodeList(fromAddress, tx.nonce))
hash.to(EthAddress)
Comment on lines -1864 to +1865
Copy link
Contributor

Choose a reason for hiding this comment

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

This one's a bit tricky, keccak returns 32 bytes and to suggests simple data type conversion. The idea here is to drop the prefix and only keep the last 20 bytes.

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/status-im/nim-eth/pull/733/files#diff-cfc06cb7a2a013ce859388f992b07f54d334dae23b5e6ae22d32faba8cc38c64R29

It "kind of" fits with the idea of a canonical conversion from Hash to Address - in the spec, it's even defined as dropping the first 12 bytes supporting the to idea and being well-defined.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it could be named something else, if you happen to have a good name handy :)


# Compute authorizations
var authorizationList = newSeqOfCap[ETHAuthorizationTuple](
Expand All @@ -1876,7 +1874,7 @@ proc ETHTransactionsCreateFromJson(
return nil
authorizationList.add ETHAuthorizationTuple(
chainId: distinctBase(auth.chainId).u256,
address: ExecutionAddress(data: auth.address),
address: ExecutionAddress(data: auth.address.data),
nonce: auth.nonce,
authority: ExecutionAddress(data: authority),
signature: @signature)
Expand All @@ -1890,14 +1888,14 @@ proc ETHTransactionsCreateFromJson(
maxFeePerGas: tx.maxFeePerGas.uint64,
gas: tx.gasLimit.uint64,
destinationType: destinationType,
to: ExecutionAddress(data: toAddress),
to: ExecutionAddress(data: toAddress.data),
value: tx.value,
input: tx.payload,
accessList: tx.accessList.mapIt(ETHAccessTuple(
address: ExecutionAddress(data: it.address),
storageKeys: it.storageKeys.mapIt(Eth2Digest(data: it)))),
address: ExecutionAddress(data: it.address.data),
storageKeys: it.storageKeys.mapIt(Eth2Digest(data: it.data)))),
maxFeePerBlobGas: tx.maxFeePerBlobGas,
blobVersionedHashes: tx.versionedHashes,
blobVersionedHashes: tx.versionedHashes.mapIt(Eth2Digest(data: it.data)),
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't this asEth2Digest? (if we want to align type names maybe Eth2Digest -> Root could be another one)

Copy link
Member Author

Choose a reason for hiding this comment

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

most of this will go away in the next set of pr:s which will reuse the new nim-eth types in nim-web3, getting rid of most of the conversion friction - this is just a "make it work" step.

authorizationList: authorizationList,
signature: @rawSig,
bytes: rlpBytes.TypedTransaction)
Expand Down Expand Up @@ -2581,14 +2579,14 @@ proc ETHReceiptsCreateFromJson(
status: distinctBase(data.status.get(1.Quantity)) != 0'u64,
hash:
if data.root.isSome:
ExecutionHash256(data: distinctBase(data.root.get))
ExecutionHash256(distinctBase(data.root.get))
else:
default(ExecutionHash256),
cumulativeGasUsed: distinctBase(data.cumulativeGasUsed).GasInt,
logsBloom: distinctBase(data.logsBloom),
logsBloom: distinctBase(data.logsBloom).to(Bloom),
logs: data.logs.mapIt(Log(
address: distinctBase(it.address),
topics: it.topics.mapIt(distinctBase(it)),
address: distinctBase(it.address).to(EthAddress),
topics: it.topics.mapIt(distinctBase(it).to(Bytes32)),
data: it.data)))
rlpBytes =
try:
Expand All @@ -2605,10 +2603,10 @@ proc ETHReceiptsCreateFromJson(
root: rec.hash,
status: rec.status,
gasUsed: distinctBase(data.gasUsed), # Validated during sanity checks.
logsBloom: BloomLogs(data: rec.logsBloom),
logsBloom: BloomLogs(data: rec.logsBloom.data),
logs: rec.logs.mapIt(ETHLog(
address: ExecutionAddress(data: it.address),
topics: it.topics.mapIt(Eth2Digest(data: it)),
address: ExecutionAddress(data: it.address.data),
topics: it.topics.mapIt(Eth2Digest(data: it.data)),
data: it.data)),
bytes: rlpBytes)

Expand Down
Loading
Loading