Skip to content

Commit 48a2407

Browse files
atoulmeatoulme
committed
Address code review comments
Signed-off-by: Antoine Toulme <[email protected]>
1 parent a517f61 commit 48a2407

File tree

28 files changed

+80
-92
lines changed

28 files changed

+80
-92
lines changed

‎besu/src/main/java/org/hyperledger/besu/chainimport/JsonBlockImporter.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ private void setOptionalFields(
121121
final GenesisConfigOptions genesisConfig) {
122122
// Some fields can only be configured for ethash
123123
if (genesisConfig.getPowAlgorithm() != PowAlgorithm.UNSUPPORTED) {
124-
// For simplicity only set these for ethash. Other consensus algorithms use these fields for
125-
// special purposes or ignore them
124+
// For simplicity only set these for PoW consensus algorithms.
125+
// Other consensus algorithms use these fields for special purposes or ignore them.
126126
miner.setCoinbase(blockData.getCoinbase().orElse(Address.ZERO));
127127
miner.setExtraData(blockData.getExtraData().orElse(Bytes.EMPTY));
128128
} else if (blockData.getCoinbase().isPresent() || blockData.getExtraData().isPresent()) {

‎besu/src/main/java/org/hyperledger/besu/cli/config/EthNetworkConfig.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ public class EthNetworkConfig {
5757
private static final String RINKEBY_GENESIS = "/rinkeby.json";
5858
private static final String GOERLI_GENESIS = "/goerli.json";
5959
private static final String DEV_GENESIS = "/dev.json";
60-
private static final String ECIP1049_DEV_GENESIS = "/ecip1049_dev.json";
60+
private static final String DEV_ECIP1049_GENESIS = "/ecip1049_dev.json";
6161
private static final String CLASSIC_GENESIS = "/classic.json";
6262
private static final String KOTTI_GENESIS = "/kotti.json";
6363
private static final String MORDOR_GENESIS = "/mordor.json";
@@ -158,7 +158,7 @@ public static EthNetworkConfig getNetworkConfig(final NetworkName networkName) {
158158
jsonConfig(CLASSIC_GENESIS), CLASSIC_NETWORK_ID, CLASSIC_BOOTSTRAP_NODES, null);
159159
case ECIP1049_DEV:
160160
return new EthNetworkConfig(
161-
jsonConfig(ECIP1049_DEV_GENESIS), ECIP1049_DEV_NETWORK_ID, new ArrayList<>(), null);
161+
jsonConfig(DEV_ECIP1049_GENESIS), ECIP1049_DEV_NETWORK_ID, new ArrayList<>(), null);
162162
case KOTTI:
163163
return new EthNetworkConfig(
164164
jsonConfig(KOTTI_GENESIS), KOTTI_NETWORK_ID, KOTTI_BOOTSTRAP_NODES, null);
@@ -200,7 +200,7 @@ public static String jsonConfig(final NetworkName network) {
200200
case DEV:
201201
return jsonConfig(DEV_GENESIS);
202202
case ECIP1049_DEV:
203-
return jsonConfig(ECIP1049_DEV_GENESIS);
203+
return jsonConfig(DEV_ECIP1049_GENESIS);
204204
case CLASSIC:
205205
return jsonConfig(CLASSIC_GENESIS);
206206
case KOTTI:

‎config/src/main/java/org/hyperledger/besu/config/PowAlgorithm.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616

1717
/** An enumeration of supported Proof-of-work algorithms. */
1818
public enum PowAlgorithm {
19-
ETHASH,
2019
UNSUPPORTED,
20+
ETHASH,
2121
KECCAK256;
2222

2323
public static PowAlgorithm fromString(final String str) {

‎ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetWork.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
6262
final byte[] dagSeed =
6363
DirectAcyclicGraphSeed.dagSeed(rawResult.getBlockNumber(), epochCalculator);
6464
final String[] result = {
65-
"0x" + BaseEncoding.base16().lowerCase().encode(rawResult.getPrePowHash()),
65+
rawResult.getPrePowHash().toHexString(),
6666
"0x" + BaseEncoding.base16().lowerCase().encode(dagSeed),
6767
rawResult.getTarget().toHexString(),
6868
Quantity.create(rawResult.getBlockNumber())

‎ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthSubmitWork.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
5555
Bytes.fromHexString(requestContext.getRequiredParameter(0, String.class)).getLong(0),
5656
requestContext.getRequiredParameter(2, Hash.class),
5757
null,
58-
Bytes.fromHexString(requestContext.getRequiredParameter(1, String.class))
59-
.toArrayUnsafe());
58+
Bytes.fromHexString(requestContext.getRequiredParameter(1, String.class)));
6059
final boolean result = miner.submitWork(solution);
6160
return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), result);
6261
} else {

‎ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetTransactionReceiptTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ public class EthGetTransactionReceiptTest {
107107
Optional.empty(),
108108
TransactionGasBudgetCalculator.frontier(),
109109
null,
110-
PoWHasher.ETHASH_LIGHT);
110+
Optional.of(PoWHasher.ETHASH_LIGHT));
111111
private final ProtocolSpec statusTransactionTypeSpec =
112112
new ProtocolSpec(
113113
"status",
@@ -133,7 +133,7 @@ public class EthGetTransactionReceiptTest {
133133
Optional.empty(),
134134
TransactionGasBudgetCalculator.frontier(),
135135
null,
136-
PoWHasher.ETHASH_LIGHT);
136+
Optional.of(PoWHasher.ETHASH_LIGHT));
137137

138138
@SuppressWarnings("unchecked")
139139
private final ProtocolSchedule protocolSchedule = mock(ProtocolSchedule.class);

‎ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthGetWorkTest.java

+4-10
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Optional;
3232

3333
import com.google.common.io.BaseEncoding;
34+
import org.apache.tuweni.bytes.Bytes;
3435
import org.apache.tuweni.units.bigints.UInt256;
3536
import org.junit.Before;
3637
import org.junit.Test;
@@ -64,8 +65,7 @@ public void shouldReturnCorrectMethodName() {
6465
public void shouldReturnCorrectResultOnGenesisDAG() {
6566
final JsonRpcRequestContext request = requestWithParams();
6667
final PoWSolverInputs values =
67-
new PoWSolverInputs(
68-
UInt256.fromHexString(hexValue), BaseEncoding.base16().lowerCase().decode(hexValue), 0);
68+
new PoWSolverInputs(UInt256.fromHexString(hexValue), Bytes.fromHexString(hexValue), 0);
6969
final String[] expectedValue = {
7070
"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
7171
"0x0000000000000000000000000000000000000000000000000000000000000000",
@@ -84,10 +84,7 @@ public void shouldReturnCorrectResultOnGenesisDAG() {
8484
public void shouldReturnCorrectResultOnHighBlockSeed() {
8585
final JsonRpcRequestContext request = requestWithParams();
8686
final PoWSolverInputs values =
87-
new PoWSolverInputs(
88-
UInt256.fromHexString(hexValue),
89-
BaseEncoding.base16().lowerCase().decode(hexValue),
90-
30000);
87+
new PoWSolverInputs(UInt256.fromHexString(hexValue), Bytes.fromHexString(hexValue), 30000);
9188

9289
final String[] expectedValue = {
9390
"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
@@ -114,10 +111,7 @@ public void shouldReturnCorrectResultOnHighBlockSeedEcip1099() {
114111
method = new EthGetWork(miningCoordinator);
115112
final JsonRpcRequestContext request = requestWithParams();
116113
final PoWSolverInputs values =
117-
new PoWSolverInputs(
118-
UInt256.fromHexString(hexValue),
119-
BaseEncoding.base16().lowerCase().decode(hexValue),
120-
60000);
114+
new PoWSolverInputs(UInt256.fromHexString(hexValue), Bytes.fromHexString(hexValue), 60000);
121115

122116
final String[] expectedValue = {
123117
"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",

‎ethereum/api/src/test/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthSubmitWorkTest.java

+6-7
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
import java.util.Optional;
3434

35-
import com.google.common.io.BaseEncoding;
3635
import org.apache.tuweni.bytes.Bytes;
3736
import org.apache.tuweni.units.bigints.UInt256;
3837
import org.junit.Before;
@@ -74,8 +73,7 @@ public void shouldFailIfNoMiningEnabled() {
7473
public void shouldFailIfMissingArguments() {
7574
final JsonRpcRequestContext request = requestWithParams();
7675
final PoWSolverInputs values =
77-
new PoWSolverInputs(
78-
UInt256.fromHexString(hexValue), BaseEncoding.base16().lowerCase().decode(hexValue), 0);
76+
new PoWSolverInputs(UInt256.fromHexString(hexValue), Bytes.fromHexString(hexValue), 0);
7977
when(miningCoordinator.getWorkDefinition()).thenReturn(Optional.of(values));
8078
assertThatThrownBy(
8179
() -> method.response(request), "Missing required json rpc parameter at index 0")
@@ -88,10 +86,11 @@ public void shouldReturnTrueIfGivenCorrectResult() {
8886
new PoWSolverInputs(
8987
UInt256.fromHexString(
9088
"0x0083126e978d4fdf3b645a1cac083126e978d4fdf3b645a1cac083126e978d4f"),
91-
new byte[] {
92-
15, -114, -104, 87, -95, -36, -17, 120, 52, 1, 124, 61, -6, -66, 78, -27, -57, 118,
93-
-18, -64, -103, -91, -74, -121, 42, 91, -14, -98, 101, 86, -43, -51
94-
},
89+
Bytes.wrap(
90+
new byte[] {
91+
15, -114, -104, 87, -95, -36, -17, 120, 52, 1, 124, 61, -6, -66, 78, -27, -57,
92+
118, -18, -64, -103, -91, -74, -121, 42, 91, -14, -98, 101, 86, -43, -51
93+
}),
9594
468);
9695

9796
final PoWSolution expectedFirstOutput =

‎ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutor.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public PoWBlockMiner createMiner(
7575
final PoWSolver solver =
7676
new PoWSolver(
7777
nonceGenerator,
78-
protocolSchedule.getByBlockNumber(parentHeader.getNumber() + 1).getPoWHasher(),
78+
protocolSchedule.getByBlockNumber(parentHeader.getNumber() + 1).getPoWHasher().get(),
7979
stratumMiningEnabled,
8080
ethHashObservers,
8181
epochCalculator);

‎ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWMiningCoordinatorTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public void miningCoordinatorIsCreatedDisabledWithNoReportableMiningStatistics()
5353
syncState,
5454
DEFAULT_REMOTE_SEALERS_LIMIT,
5555
DEFAULT_REMOTE_SEALERS_TTL);
56-
final PoWSolution solution = new PoWSolution(1L, Hash.EMPTY, null, new byte[Bytes32.SIZE]);
56+
final PoWSolution solution = new PoWSolution(1L, Hash.EMPTY, null, Bytes32.ZERO);
5757

5858
assertThat(miningCoordinator.isMining()).isFalse();
5959
assertThat(miningCoordinator.hashesPerSecond()).isEqualTo(Optional.empty());

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/EthHash.java

+7-5
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
import com.google.common.primitives.Ints;
3131
import com.google.common.primitives.Longs;
32+
import org.apache.tuweni.bytes.Bytes;
3233
import org.apache.tuweni.bytes.Bytes32;
3334
import org.bouncycastle.jcajce.provider.digest.Keccak;
3435

@@ -75,18 +76,18 @@ public final class EthHash {
7576
* bytes 32 to 63
7677
*/
7778
public static PoWSolution hashimotoLight(
78-
final long size, final int[] cache, final byte[] header, final long nonce) {
79+
final long size, final int[] cache, final Bytes header, final long nonce) {
7980
return hashimoto(header, size, nonce, (target, ind) -> calcDatasetItem(target, cache, ind));
8081
}
8182

8283
public static PoWSolution hashimoto(
83-
final byte[] header,
84+
final Bytes header,
8485
final long size,
8586
final long nonce,
8687
final BiConsumer<byte[], Integer> datasetLookup) {
8788
final int n = (int) Long.divideUnsigned(size, MIX_BYTES);
8889
final MessageDigest keccak512 = KECCAK_512.get();
89-
keccak512.update(header);
90+
keccak512.update(header.toArrayUnsafe());
9091
keccak512.update(Longs.toByteArray(Long.reverseBytes(nonce)));
9192
final byte[] seed = keccak512.digest();
9293
final ByteBuffer mixBuffer = ByteBuffer.allocate(MIX_BYTES).order(ByteOrder.LITTLE_ENDIAN);
@@ -171,7 +172,7 @@ public static void calcDatasetItem(final byte[] buffer, final int[] cache, final
171172
* @param header Block Header
172173
* @return Truncated BlockHeader hash
173174
*/
174-
public static byte[] hashHeader(final SealableBlockHeader header) {
175+
public static Bytes32 hashHeader(final SealableBlockHeader header) {
175176
final BytesValueRLPOutput out = new BytesValueRLPOutput();
176177
out.startList();
177178
out.writeBytes(header.getParentHash());
@@ -192,7 +193,8 @@ public static byte[] hashHeader(final SealableBlockHeader header) {
192193
out.writeLongScalar(header.getBaseFee().get());
193194
}
194195
out.endList();
195-
return DirectAcyclicGraphSeed.KECCAK_256.get().digest(out.encoded().toArray());
196+
return Bytes32.wrap(
197+
DirectAcyclicGraphSeed.KECCAK_256.get().digest(out.encoded().toArrayUnsafe()));
196198
}
197199

198200
/**

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/KeccakHasher.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,10 @@ public PoWSolution hash(
4444
final long nonce,
4545
final long number,
4646
final EpochCalculator epochCalc,
47-
final byte[] prePowHash) {
47+
final Bytes prePowHash) {
4848

4949
MessageDigest digest = KECCAK_256.get();
50-
digest.update(prePowHash);
50+
digest.update(prePowHash.toArrayUnsafe());
5151
digest.update(Bytes.ofUnsignedLong(nonce).toArrayUnsafe());
5252
Bytes32 solution = Bytes32.wrap(digest.digest());
5353
Hash mixHash = Hash.wrap(solution);

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PoWHasher.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
*/
1515
package org.hyperledger.besu.ethereum.mainnet;
1616

17+
import org.apache.tuweni.bytes.Bytes;
18+
1719
public interface PoWHasher {
1820

1921
PoWHasher ETHASH_LIGHT = new EthashLight();
@@ -28,7 +30,7 @@ public interface PoWHasher {
2830
* @param prePowHash Block Header (without mix digest and nonce) Hash
2931
* @return the PoW solution computed by the hashing function
3032
*/
31-
PoWSolution hash(long nonce, long number, EpochCalculator epochCalc, byte[] prePowHash);
33+
PoWSolution hash(long nonce, long number, EpochCalculator epochCalc, Bytes prePowHash);
3234

3335
/** Implementation of Ethash Hashimoto Light Implementation. */
3436
final class EthashLight implements PoWHasher {
@@ -42,7 +44,7 @@ public PoWSolution hash(
4244
final long nonce,
4345
final long number,
4446
final EpochCalculator epochCalc,
45-
final byte[] prePowHash) {
47+
final Bytes prePowHash) {
4648
final EthHashCacheFactory.EthHashDescriptor cache =
4749
cacheFactory.ethHashCacheFor(number, epochCalc);
4850
final PoWSolution solution =
@@ -61,7 +63,7 @@ public PoWSolution hash(
6163
final long nonce,
6264
final long number,
6365
final EpochCalculator epochCalc,
64-
final byte[] prePowHash) {
66+
final Bytes prePowHash) {
6567
throw new UnsupportedOperationException("Hashing is unsupported");
6668
}
6769
}

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PoWSolution.java

+8-11
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,18 @@
1616

1717
import org.hyperledger.besu.ethereum.core.Hash;
1818

19-
import java.util.Arrays;
2019
import java.util.Objects;
2120

22-
import org.apache.tuweni.bytes.Bytes32;
21+
import org.apache.tuweni.bytes.Bytes;
2322

2423
public class PoWSolution {
2524
private final long nonce;
2625
private final Hash mixHash;
27-
private final byte[] powHash;
28-
private final Bytes32 solution;
26+
private final Bytes powHash;
27+
private final Bytes solution;
2928

3029
public PoWSolution(
31-
final long nonce, final Hash mixHash, final Bytes32 solution, final byte[] powHash) {
30+
final long nonce, final Hash mixHash, final Bytes solution, final Bytes powHash) {
3231
this.nonce = nonce;
3332
this.mixHash = mixHash;
3433
this.solution = solution;
@@ -43,11 +42,11 @@ public Hash getMixHash() {
4342
return mixHash;
4443
}
4544

46-
public byte[] getPowHash() {
45+
public Bytes getPowHash() {
4746
return powHash;
4847
}
4948

50-
public Bytes32 getSolution() {
49+
public Bytes getSolution() {
5150
return solution;
5251
}
5352

@@ -59,13 +58,11 @@ public boolean equals(final Object o) {
5958
return nonce == that.nonce
6059
&& Objects.equals(mixHash, that.mixHash)
6160
&& Objects.equals(solution, that.solution)
62-
&& Arrays.equals(powHash, that.powHash);
61+
&& Objects.equals(powHash, that.powHash);
6362
}
6463

6564
@Override
6665
public int hashCode() {
67-
int result = Objects.hash(nonce, mixHash, solution);
68-
result = 31 * result + Arrays.hashCode(powHash);
69-
return result;
66+
return Objects.hash(nonce, mixHash, solution, powHash);
7067
}
7168
}

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PoWSolver.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import org.hyperledger.besu.ethereum.chain.PoWObserver;
2020
import org.hyperledger.besu.util.Subscribers;
2121

22-
import java.util.Arrays;
2322
import java.util.Optional;
2423
import java.util.concurrent.CompletableFuture;
2524
import java.util.concurrent.ExecutionException;
@@ -161,7 +160,7 @@ public boolean submitSolution(final PoWSolution solution) {
161160

162161
final PoWSolverJob job = jobSnapshot.get();
163162
final PoWSolverInputs inputs = job.getInputs();
164-
if (!Arrays.equals(inputs.getPrePowHash(), solution.getPowHash())) {
163+
if (!inputs.getPrePowHash().equals(solution.getPowHash())) {
165164
LOG.debug("Miner's solution does not match current job");
166165
return false;
167166
}

‎ethereum/core/src/main/java/org/hyperledger/besu/ethereum/mainnet/PoWSolverInputs.java

+6-7
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,15 @@
1414
*/
1515
package org.hyperledger.besu.ethereum.mainnet;
1616

17-
import java.util.Arrays;
18-
17+
import org.apache.tuweni.bytes.Bytes;
1918
import org.apache.tuweni.units.bigints.UInt256;
2019

2120
public class PoWSolverInputs {
2221
private final UInt256 target;
23-
private final byte[] prePowHash;
22+
private final Bytes prePowHash;
2423
private final long blockNumber;
2524

26-
public PoWSolverInputs(final UInt256 target, final byte[] prePowHash, final long blockNumber) {
25+
public PoWSolverInputs(final UInt256 target, final Bytes prePowHash, final long blockNumber) {
2726
this.target = target;
2827
this.prePowHash = prePowHash;
2928
this.blockNumber = blockNumber;
@@ -33,7 +32,7 @@ public UInt256 getTarget() {
3332
return target;
3433
}
3534

36-
public byte[] getPrePowHash() {
35+
public Bytes getPrePowHash() {
3736
return prePowHash;
3837
}
3938

@@ -43,11 +42,11 @@ public long getBlockNumber() {
4342

4443
@Override
4544
public String toString() {
46-
return "MiningSolverInputs{"
45+
return "PoWSolverInputs{"
4746
+ "target="
4847
+ target
4948
+ ", prePowHash="
50-
+ Arrays.toString(prePowHash)
49+
+ prePowHash
5150
+ ", blockNumber="
5251
+ blockNumber
5352
+ '}';

0 commit comments

Comments
 (0)