EIP-6110: Add deposits in EL #5055
There are no files selected for viewing
| 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
There was a problem hiding this comment. 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. Yea I was debating whether to put it under There was a problem hiding this comment. 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. 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 There was a problem hiding this comment.
I agree we should just remove There was a problem hiding this comment. 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. 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.
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. |
||
|
|
||
| /** 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)); | ||
| } | ||
| } | ||
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 think the deposits are derived from the transaction receipt logs after the transactions are processed and so should be generated within createBlock rather than passed in.
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.
Yeah doesn't make sense to pass in deposits as these are derived from the state after processing the transactions
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.
If I am not wrong I think in
AbstractBlockCreator.createBlock()I will get the deposits fromtransactionResultsand stuff them into the block header and body. But this involves ABI decoding which will be included in the next PR so for now I am gonna leave the deposits to be empty and add a TODO here