-
Notifications
You must be signed in to change notification settings - Fork 897
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
Changes from all commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
d57d70b
Initial commit
ensi321 1c8de5a
Merge branch 'main' into eip-6110
ensi321 559b12a
Added object classes for deposit fields
ensi321 22d4aa4
Spotless apply
ensi321 97289b2
Fix up compilation issue
ensi321 1247429
Fix up compilation issue
ensi321 2e68cca
Fix up compilation issue
ensi321 91a851f
Update existed tests
ensi321 e0eb573
Merge branch 'main' into eip-6110
ensi321 e161250
Remove feature flag for this EIP for now
ensi321 ba3e19a
Renmae WithdrawalCredential
ensi321 0ca48a0
Remove DepositProcessor
ensi321 8707fc0
Remove all certain validation on deposits
ensi321 bb66d97
Remove all uncertain validation on deposits
ensi321 6837699
Merge remote-tracking branch 'remote/main' into eip-6110
ensi321 ff28502
Fix test
ensi321 ae25f12
Fix typo
ensi321 672a745
Fix test
ensi321 307582d
Update plugin-api hash
ensi321 d8da6b7
Partial commit
ensi321 2e07bb7
Partial commit
ensi321 48cda2a
Added unit test
ensi321 f645938
SpotlessApply
ensi321 cae7435
Merge branch 'main' into eip-6110
ensi321 8cfa587
Fix comment
ensi321 30d3083
Rename TODO and isExperimentalEipsTime in GenesisState
ensi321 225b56d
Update copyright info
ensi321 17ce42b
Remove deposits from createBlock() param
ensi321 15fd353
Add unstable annotation
ensi321 ba461ce
Put deposits root after excess gas in BlockHeader
ensi321 8ae831f
Add deposits root to GetBodiesFromPeerTask.BodyIdentifier
ensi321 3470caa
Update tests
ensi321 7065425
Remove DepositWithdrawalCredential
ensi321 6995414
Update test
ensi321 7d3fc83
Update test
ensi321 46fa21a
Merge remote-tracking branch 'remote/main' into eip-6110
ensi321 ed6f99f
Update test
ensi321 bce7b56
Apply spotless
ensi321 e8f5b96
Update hash
ensi321 33adc69
Fix up some test
ensi321 da838f6
Add DepositsRoot test and related block files
ensi321 4efac87
Merge remote-tracking branch 'remote/main' into eip-6110
ensi321 34453d4
Fix conflict
ensi321 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
83 changes: 83 additions & 0 deletions
83
datatypes/src/main/java/org/hyperledger/besu/datatypes/BLSPublicKey.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 { | ||
|
||
/** 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)); | ||
} | ||
} |
83 changes: 83 additions & 0 deletions
83
datatypes/src/main/java/org/hyperledger/besu/datatypes/BLSSignature.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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)); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
144 changes: 144 additions & 0 deletions
144
.../java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/parameters/DepositParameter.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 | ||
+ '\'' | ||
+ '}'; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
orcrypto
. My line of thought iscrypto
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 forBLSPublicKey
hence putting it underdatatypes
. I am happy to move it tocrypto
if you think otherwise.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should just remove
DepositWithdrawalCredential
and make the field as Bytes32. What do you think aboutBLSPublicKey
andBLSSignature
? 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 sugarThere was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Yea I agree. We can move them to Deposit.