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.

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

EIP-6110: Add deposits in EL #5055

Merged
merged 43 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from 40 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
d57d70b
Initial commit
ensi321 Feb 5, 2023
1c8de5a
Merge branch 'main' into eip-6110
ensi321 Feb 19, 2023
559b12a
Added object classes for deposit fields
ensi321 Feb 19, 2023
22d4aa4
Spotless apply
ensi321 Feb 19, 2023
97289b2
Fix up compilation issue
ensi321 Feb 19, 2023
1247429
Fix up compilation issue
ensi321 Feb 19, 2023
2e68cca
Fix up compilation issue
ensi321 Feb 20, 2023
91a851f
Update existed tests
ensi321 Feb 20, 2023
e0eb573
Merge branch 'main' into eip-6110
ensi321 Feb 20, 2023
e161250
Remove feature flag for this EIP for now
ensi321 Feb 23, 2023
ba3e19a
Renmae WithdrawalCredential
ensi321 Feb 25, 2023
0ca48a0
Remove DepositProcessor
ensi321 Feb 25, 2023
8707fc0
Remove all certain validation on deposits
ensi321 Feb 25, 2023
bb66d97
Remove all uncertain validation on deposits
ensi321 Feb 25, 2023
6837699
Merge remote-tracking branch 'remote/main' into eip-6110
ensi321 Feb 25, 2023
ff28502
Fix test
ensi321 Feb 25, 2023
ae25f12
Fix typo
ensi321 Mar 1, 2023
672a745
Fix test
ensi321 Mar 3, 2023
307582d
Update plugin-api hash
ensi321 Mar 3, 2023
d8da6b7
Partial commit
ensi321 Mar 4, 2023
2e07bb7
Partial commit
ensi321 Mar 5, 2023
48cda2a
Added unit test
ensi321 Mar 5, 2023
f645938
SpotlessApply
ensi321 Mar 6, 2023
cae7435
Merge branch 'main' into eip-6110
ensi321 Mar 6, 2023
8cfa587
Fix comment
ensi321 Mar 6, 2023
30d3083
Rename TODO and isExperimentalEipsTime in GenesisState
ensi321 Mar 9, 2023
225b56d
Update copyright info
ensi321 Mar 9, 2023
17ce42b
Remove deposits from createBlock() param
ensi321 Mar 10, 2023
15fd353
Add unstable annotation
ensi321 Mar 11, 2023
ba461ce
Put deposits root after excess gas in BlockHeader
ensi321 Mar 11, 2023
8ae831f
Add deposits root to GetBodiesFromPeerTask.BodyIdentifier
ensi321 Mar 11, 2023
3470caa
Update tests
ensi321 Mar 12, 2023
7065425
Remove DepositWithdrawalCredential
ensi321 Mar 19, 2023
6995414
Update test
ensi321 Mar 19, 2023
7d3fc83
Update test
ensi321 Mar 19, 2023
46fa21a
Merge remote-tracking branch 'remote/main' into eip-6110
ensi321 Mar 19, 2023
ed6f99f
Update test
ensi321 Mar 19, 2023
bce7b56
Apply spotless
ensi321 Mar 19, 2023
e8f5b96
Update hash
ensi321 Mar 19, 2023
33adc69
Fix up some test
ensi321 Mar 19, 2023
da838f6
Add DepositsRoot test and related block files
ensi321 Mar 26, 2023
4efac87
Merge remote-tracking branch 'remote/main' into eip-6110
ensi321 Mar 28, 2023
34453d4
Fix conflict
ensi321 Mar 28, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public static BlockHeader createBlockHeader(
new BigInteger(block.getNonceRaw().substring(2), 16).longValue(),
null,
null,
null,
blockHeaderFunctions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ private void setSyncTarget() {
mock(EthPeer.class),
new org.hyperledger.besu.ethereum.core.BlockHeader(
null, null, null, null, null, null, null, null, 1, 1, 1, 1, null, null, null, 1, null,
null, null));
null, null, null));
}

private void clearSyncTarget() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public BlockCreationResult createBlock(
final Bytes32 random,
final long timestamp,
final Optional<List<Withdrawal>> withdrawals) {

return createBlock(
maybeTransactions,
Optional.of(Collections.emptyList()),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright contributors to Hyperledger Besu
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.datatypes;

import static com.google.common.base.Preconditions.checkArgument;

import org.hyperledger.besu.ethereum.rlp.RLPException;
import org.hyperledger.besu.ethereum.rlp.RLPInput;

import com.fasterxml.jackson.annotation.JsonCreator;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.DelegatingBytes;

/** A BLS public key. */
public class BLSPublicKey extends DelegatingBytes
implements org.hyperledger.besu.plugin.data.PublicKey {
Comment on lines +27 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

What's your thinking with respect to the inheritance hierarchy here?

I would need to understand the class hierarchy a bit more myself, but just throwing out another possible place for this to live: https://github.com/hyperledger/besu/tree/main/crypto/src/main/java/org/hyperledger/besu/crypto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea I was debating whether to put it under datatypes or crypto. My line of thought is crypto is more dedicated to classes that need crypto-related operations such as encryption and such. BLSPublicKey is mainly a CL thing, all we are doing in EL is just view this as a 48-byte field and we don't perform any crypto operation for BLSPublicKey hence putting it under datatypes. I am happy to move it to crypto if you think otherwise.

Copy link
Contributor

@siladu siladu Mar 8, 2023

Choose a reason for hiding this comment

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

I want to have a chat with someone who knows this area of besu better, or wait until another maintainer reviews to get a second opinion about the appropriate location for this and the other new datatypes.

Copy link
Contributor

Choose a reason for hiding this comment

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

The datatypes package does seem like a more appropriate place than crypto as it is more focused on operations.

However, the intent of the datatype module is for broad-based types. So I don't think the DepositWithdrawalCredential would qualify and should probably just be a Bytes32 since we only care about bytes anyway or move it closer to where it is needed rather than in the top-level module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The datatypes package does seem like a more appropriate place than crypto as it is more focused on operations.

However, the intent of the datatype module is for broad-based types. So I don't think the DepositWithdrawalCredential would qualify and should probably just be a Bytes32 since we only care about bytes anyway or move it closer to where it is needed rather than in the top-level module.

I agree we should just remove DepositWithdrawalCredential and make the field as Bytes32. What do you think about BLSPublicKey and BLSSignature? AFAIK we are not doing anything special for these 3 fields as we only care about the bytes. Having dedicated data classes are kind of like syntactic sugar

Copy link
Contributor

Choose a reason for hiding this comment

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

I think BLSPublicKey and BLSSignature are OK as new types since they are quite a general concept and could imagine them being reused in the future.
Whereas DepositWithdrawalsCredential is quite specific Bytes32 is better IMO...it's not clear to me from the EIP what format this credential takes exactly, maybe there's an existing data type that is appropriate?

Possibly BLSPublicKey and BLSSignature shouldn't live in data types though since they are not broadly used. Maybe should live alongside Deposit.java for now?

Copy link
Contributor Author

@ensi321 ensi321 Mar 21, 2023

Choose a reason for hiding this comment

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

I think BLSPublicKey and BLSSignature are OK as new types since they are quite a general concept and could imagine them being reused in the future. Whereas DepositWithdrawalsCredential is quite specific Bytes32 is better IMO...it's not clear to me from the EIP what format this credential takes exactly, maybe there's an existing data type that is appropriate?

The format of withdrawal credential is one of its kind and is rather specific. To summarize what it contains: there are two types one with 0x00 and one with 0x01 prefix. The first type is a hash of public key with the first byte being replaced by 0. The second type is simply a 20-byte eth1 address. I think there are some discussions in CL on coming up with new prefixes and new encoding for the withdrawal credential. So in short, I don't think we can use existing data type for it.

Possibly BLSPublicKey and BLSSignature shouldn't live in data types though since they are not broadly used. Maybe should live alongside Deposit.java for now?

Yea I agree. We can move them to Deposit.


/** The constant SIZE. */
public static final int SIZE = 48;

/**
* Instantiates a new BLSPublicKey.
*
* @param bytes the bytes
*/
protected BLSPublicKey(final Bytes bytes) {
super(bytes);
}

/**
* Wrap public key.
*
* @param value the value
* @return the BLS public key
*/
public static BLSPublicKey wrap(final Bytes value) {
checkArgument(
value.size() == SIZE, "A BLS public key must be %s bytes long, got %s", SIZE, value.size());
return new BLSPublicKey(value);
}

/**
* Creates a bls public key from the given RLP-encoded input.
*
* @param input The input to read from
* @return the input's corresponding public key
*/
public static BLSPublicKey readFrom(final RLPInput input) {
final Bytes bytes = input.readBytes();
if (bytes.size() != SIZE) {
throw new RLPException(
String.format("BLSPublicKey unexpected size of %s (needs %s)", bytes.size(), SIZE));
}
return BLSPublicKey.wrap(bytes);
}

/**
* Parse a hexadecimal string representing an public key.
*
* @param str A hexadecimal string (with or without the leading '0x') representing a valid bls
* public key.
* @return The parsed bls public key: {@code null} if the provided string is {@code null}.
* @throws IllegalArgumentException if the string is either not hexadecimal, or not the valid
* representation of a BLSPublicKey.
*/
@JsonCreator
public static BLSPublicKey fromHexString(final String str) {
if (str == null) return null;
return wrap(Bytes.fromHexStringLenient(str, SIZE));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright contributors to Hyperledger Besu
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.datatypes;

import static com.google.common.base.Preconditions.checkArgument;

import org.hyperledger.besu.ethereum.rlp.RLPException;
import org.hyperledger.besu.ethereum.rlp.RLPInput;

import com.fasterxml.jackson.annotation.JsonCreator;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.DelegatingBytes;

/** A BLS Signature * */
public class BLSSignature extends DelegatingBytes
implements org.hyperledger.besu.plugin.data.Signature {

/** The constant SIZE. */
public static final int SIZE = 96;

/**
* Instantiates a new BLSSignature.
*
* @param bytes the bytes
*/
protected BLSSignature(final Bytes bytes) {
super(bytes);
}

/**
* Wrap signature.
*
* @param value the value
* @return the signature
*/
public static BLSSignature wrap(final Bytes value) {
checkArgument(
value.size() == SIZE, "A signature must be %s bytes long, got %s", SIZE, value.size());
return new BLSSignature(value);
}

/**
* Creates a signature from the given RLP-encoded input.
*
* @param input The input to read from
* @return the input's corresponding signature
*/
public static BLSSignature readFrom(final RLPInput input) {
final Bytes bytes = input.readBytes();
if (bytes.size() != SIZE) {
throw new RLPException(
String.format("BLSSignature unexpected size of %s (needs %s)", bytes.size(), SIZE));
}
return BLSSignature.wrap(bytes);
}

/**
* Parse a hexadecimal string representing a signature.
*
* @param str A hexadecimal string (with or without the leading '0x') representing a valid bls
* signature.
* @return The parsed signature: {@code null} if the provided string is {@code null}.
* @throws IllegalArgumentException if the string is either not hexadecimal, or not the valid
* representation of a bls signature.
*/
@JsonCreator
public static BLSSignature fromHexString(final String str) {
if (str == null) return null;
return wrap(Bytes.fromHexStringLenient(str, SIZE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,6 @@ public enum JsonRpcResponseKey {
TOTAL_DIFFICULTY,
TRANSACTION_ROOT,
BASEFEE,
WITHDRAWALS_ROOT
WITHDRAWALS_ROOT,
DEPOSITS_ROOT
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcResponseKey.BASEFEE;
import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcResponseKey.COINBASE;
import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcResponseKey.DEPOSITS_ROOT;
import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcResponseKey.DIFFICULTY;
import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcResponseKey.EXTRA_DATA;
import static org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcResponseKey.GAS_LIMIT;
Expand Down Expand Up @@ -103,6 +104,8 @@ public JsonRpcResponse response(
final int size = unsignedInt(values.get(SIZE));
final Hash withdrawalsRoot =
values.containsKey(WITHDRAWALS_ROOT) ? hash(values.get(WITHDRAWALS_ROOT)) : null;
final Hash depositsRoot =
values.containsKey(DEPOSITS_ROOT) ? hash(values.get(DEPOSITS_ROOT)) : null;
final List<JsonNode> ommers = new ArrayList<>();

final BlockHeader header =
Expand All @@ -125,6 +128,7 @@ public JsonRpcResponse response(
nonce,
withdrawalsRoot,
null, // ToDo 4844: set with the value of excess_data_gas field
depositsRoot,
blockHeaderFunctions);

return new JsonRpcSuccessResponse(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
0,
maybeWithdrawals.map(BodyValidation::withdrawalsRoot).orElse(null),
null,
null,
headerFunctions);

// ensure the block hash matches the blockParam hash
Expand Down Expand Up @@ -201,7 +202,9 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)

final var block =
new Block(
newBlockHeader, new BlockBody(transactions, Collections.emptyList(), maybeWithdrawals));
newBlockHeader,
new BlockBody(
transactions, Collections.emptyList(), maybeWithdrawals, Optional.empty()));

if (parentHeader.isEmpty()) {
LOG.atDebug()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
/*
* Copyright contributors to Hyperledger Besu
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
* specific language governing permissions and limitations under the License.
*
* SPDX-License-Identifier: Apache-2.0
*/
package org.hyperledger.besu.ethereum.api.jsonrpc.internal.parameters;

import org.hyperledger.besu.datatypes.BLSPublicKey;
import org.hyperledger.besu.datatypes.BLSSignature;
import org.hyperledger.besu.datatypes.GWei;
import org.hyperledger.besu.ethereum.core.Deposit;

import java.util.Objects;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonGetter;
import com.fasterxml.jackson.annotation.JsonProperty;
import io.vertx.core.json.JsonObject;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt64;

public class DepositParameter {

private final String publicKey;

private final String withdrawalCredentials;
private final String amount;

private final String signature;
private final String index;

@JsonCreator
public DepositParameter(
@JsonProperty("pubKey") final String pubKey,
@JsonProperty("withdrawal_credentials") final String withdrawalCredentials,
@JsonProperty("amount") final String amount,
@JsonProperty("signature") final String signature,
@JsonProperty("index") final String index) {
this.publicKey = pubKey;
this.withdrawalCredentials = withdrawalCredentials;
this.amount = amount;
this.signature = signature;
this.index = index;
}

public static DepositParameter fromDeposit(final Deposit deposit) {
return new DepositParameter(
deposit.getPublicKey().toString(),
deposit.getWithdrawalCredentials().toString(),
deposit.getAmount().toShortHexString(),
deposit.getSignature().toString(),
deposit.getIndex().toBytes().toQuantityHexString());
}

public Deposit toDeposit() {
return new Deposit(
BLSPublicKey.fromHexString(publicKey),
Bytes32.fromHexString(withdrawalCredentials),
GWei.fromHexString(amount),
BLSSignature.fromHexString(signature),
UInt64.fromHexString(index));
}

public JsonObject asJsonObject() {
return new JsonObject()
.put("pubKey", publicKey)
.put("withdrawalCredentials", withdrawalCredentials)
.put("amount", amount)
.put("signature", signature)
.put("index", index);
}

@JsonGetter
public String getPublicKey() {
return publicKey;
}

@JsonGetter
public String getWithdrawalCredentials() {
return withdrawalCredentials;
}

@JsonGetter
public String getAmount() {
return amount;
}

@JsonGetter
public String getSignature() {
return signature;
}

@JsonGetter
public String getIndex() {
return index;
}

@Override
public boolean equals(final Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
final DepositParameter that = (DepositParameter) o;
return Objects.equals(publicKey, that.publicKey)
&& Objects.equals(withdrawalCredentials, that.withdrawalCredentials)
&& Objects.equals(amount, that.amount)
&& Objects.equals(signature, that.signature)
&& Objects.equals(index, that.index);
}

@Override
public int hashCode() {
return Objects.hash(publicKey, withdrawalCredentials, amount, signature, index);
}

@Override
public String toString() {
return "DepositParameter{"
+ "pubKey='"
+ publicKey
+ '\''
+ ", withdrawalCredentials='"
+ withdrawalCredentials
+ '\''
+ ", amount='"
+ amount
+ '\''
+ ", signature='"
+ signature
+ '\''
+ ", index='"
+ index
+ '\''
+ '}';
}
}
Loading