Skip to content

Commit

Permalink
Address code review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Antoine Toulme <[email protected]>
  • Loading branch information
atoulme committed Mar 2, 2021
1 parent a517f61 commit bc70025
Show file tree
Hide file tree
Showing 28 changed files with 79 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ private void setOptionalFields(
final GenesisConfigOptions genesisConfig) {
// Some fields can only be configured for ethash
if (genesisConfig.getPowAlgorithm() != PowAlgorithm.UNSUPPORTED) {
// For simplicity only set these for ethash. Other consensus algorithms use these fields for
// special purposes or ignore them
// For simplicity only set these for PoW consensus algorithms.
// Other consensus algorithms use these fields for special purposes or ignore them.
miner.setCoinbase(blockData.getCoinbase().orElse(Address.ZERO));
miner.setExtraData(blockData.getExtraData().orElse(Bytes.EMPTY));
} else if (blockData.getCoinbase().isPresent() || blockData.getExtraData().isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class EthNetworkConfig {
private static final String RINKEBY_GENESIS = "/rinkeby.json";
private static final String GOERLI_GENESIS = "/goerli.json";
private static final String DEV_GENESIS = "/dev.json";
private static final String ECIP1049_DEV_GENESIS = "/ecip1049_dev.json";
private static final String DEV_ECIP1049_GENESIS = "/ecip1049_dev.json";
private static final String CLASSIC_GENESIS = "/classic.json";
private static final String KOTTI_GENESIS = "/kotti.json";
private static final String MORDOR_GENESIS = "/mordor.json";
Expand Down Expand Up @@ -158,7 +158,7 @@ public static EthNetworkConfig getNetworkConfig(final NetworkName networkName) {
jsonConfig(CLASSIC_GENESIS), CLASSIC_NETWORK_ID, CLASSIC_BOOTSTRAP_NODES, null);
case ECIP1049_DEV:
return new EthNetworkConfig(
jsonConfig(ECIP1049_DEV_GENESIS), ECIP1049_DEV_NETWORK_ID, new ArrayList<>(), null);
jsonConfig(DEV_ECIP1049_GENESIS), ECIP1049_DEV_NETWORK_ID, new ArrayList<>(), null);
case KOTTI:
return new EthNetworkConfig(
jsonConfig(KOTTI_GENESIS), KOTTI_NETWORK_ID, KOTTI_BOOTSTRAP_NODES, null);
Expand Down Expand Up @@ -200,7 +200,7 @@ public static String jsonConfig(final NetworkName network) {
case DEV:
return jsonConfig(DEV_GENESIS);
case ECIP1049_DEV:
return jsonConfig(ECIP1049_DEV_GENESIS);
return jsonConfig(DEV_ECIP1049_GENESIS);
case CLASSIC:
return jsonConfig(CLASSIC_GENESIS);
case KOTTI:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@

/** An enumeration of supported Proof-of-work algorithms. */
public enum PowAlgorithm {
ETHASH,
UNSUPPORTED,
ETHASH,
KECCAK256;

public static PowAlgorithm fromString(final String str) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
final byte[] dagSeed =
DirectAcyclicGraphSeed.dagSeed(rawResult.getBlockNumber(), epochCalculator);
final String[] result = {
"0x" + BaseEncoding.base16().lowerCase().encode(rawResult.getPrePowHash()),
rawResult.getPrePowHash().toHexString(),
"0x" + BaseEncoding.base16().lowerCase().encode(dagSeed),
rawResult.getTarget().toHexString(),
Quantity.create(rawResult.getBlockNumber())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
Bytes.fromHexString(requestContext.getRequiredParameter(0, String.class)).getLong(0),
requestContext.getRequiredParameter(2, Hash.class),
null,
Bytes.fromHexString(requestContext.getRequiredParameter(1, String.class))
.toArrayUnsafe());
Bytes.fromHexString(requestContext.getRequiredParameter(1, String.class)));
final boolean result = miner.submitWork(solution);
return new JsonRpcSuccessResponse(requestContext.getRequest().getId(), result);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public class EthGetTransactionReceiptTest {
Optional.empty(),
TransactionGasBudgetCalculator.frontier(),
null,
PoWHasher.ETHASH_LIGHT);
Optional.of(PoWHasher.ETHASH_LIGHT));
private final ProtocolSpec statusTransactionTypeSpec =
new ProtocolSpec(
"status",
Expand All @@ -133,7 +133,7 @@ public class EthGetTransactionReceiptTest {
Optional.empty(),
TransactionGasBudgetCalculator.frontier(),
null,
PoWHasher.ETHASH_LIGHT);
Optional.of(PoWHasher.ETHASH_LIGHT));

@SuppressWarnings("unchecked")
private final ProtocolSchedule protocolSchedule = mock(ProtocolSchedule.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Optional;

import com.google.common.io.BaseEncoding;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.units.bigints.UInt256;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -64,8 +65,7 @@ public void shouldReturnCorrectMethodName() {
public void shouldReturnCorrectResultOnGenesisDAG() {
final JsonRpcRequestContext request = requestWithParams();
final PoWSolverInputs values =
new PoWSolverInputs(
UInt256.fromHexString(hexValue), BaseEncoding.base16().lowerCase().decode(hexValue), 0);
new PoWSolverInputs(UInt256.fromHexString(hexValue), Bytes.fromHexString(hexValue), 0);
final String[] expectedValue = {
"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
"0x0000000000000000000000000000000000000000000000000000000000000000",
Expand All @@ -84,10 +84,7 @@ public void shouldReturnCorrectResultOnGenesisDAG() {
public void shouldReturnCorrectResultOnHighBlockSeed() {
final JsonRpcRequestContext request = requestWithParams();
final PoWSolverInputs values =
new PoWSolverInputs(
UInt256.fromHexString(hexValue),
BaseEncoding.base16().lowerCase().decode(hexValue),
30000);
new PoWSolverInputs(UInt256.fromHexString(hexValue), Bytes.fromHexString(hexValue), 30000);

final String[] expectedValue = {
"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
Expand All @@ -114,10 +111,7 @@ public void shouldReturnCorrectResultOnHighBlockSeedEcip1099() {
method = new EthGetWork(miningCoordinator);
final JsonRpcRequestContext request = requestWithParams();
final PoWSolverInputs values =
new PoWSolverInputs(
UInt256.fromHexString(hexValue),
BaseEncoding.base16().lowerCase().decode(hexValue),
60000);
new PoWSolverInputs(UInt256.fromHexString(hexValue), Bytes.fromHexString(hexValue), 60000);

final String[] expectedValue = {
"0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@

import java.util.Optional;

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

final PoWSolution expectedFirstOutput =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public PoWBlockMiner createMiner(
final PoWSolver solver =
new PoWSolver(
nonceGenerator,
protocolSchedule.getByBlockNumber(parentHeader.getNumber() + 1).getPoWHasher(),
protocolSchedule.getByBlockNumber(parentHeader.getNumber() + 1).getPoWHasher().get(),
stratumMiningEnabled,
ethHashObservers,
epochCalculator);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void miningCoordinatorIsCreatedDisabledWithNoReportableMiningStatistics()
syncState,
DEFAULT_REMOTE_SEALERS_LIMIT,
DEFAULT_REMOTE_SEALERS_TTL);
final PoWSolution solution = new PoWSolution(1L, Hash.EMPTY, null, new byte[Bytes32.SIZE]);
final PoWSolution solution = new PoWSolution(1L, Hash.EMPTY, null, Bytes32.ZERO);

assertThat(miningCoordinator.isMining()).isFalse();
assertThat(miningCoordinator.hashesPerSecond()).isEqualTo(Optional.empty());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import com.google.common.primitives.Ints;
import com.google.common.primitives.Longs;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.bouncycastle.jcajce.provider.digest.Keccak;

Expand Down Expand Up @@ -75,18 +76,18 @@ public final class EthHash {
* bytes 32 to 63
*/
public static PoWSolution hashimotoLight(
final long size, final int[] cache, final byte[] header, final long nonce) {
final long size, final int[] cache, final Bytes header, final long nonce) {
return hashimoto(header, size, nonce, (target, ind) -> calcDatasetItem(target, cache, ind));
}

public static PoWSolution hashimoto(
final byte[] header,
final Bytes header,
final long size,
final long nonce,
final BiConsumer<byte[], Integer> datasetLookup) {
final int n = (int) Long.divideUnsigned(size, MIX_BYTES);
final MessageDigest keccak512 = KECCAK_512.get();
keccak512.update(header);
keccak512.update(header.toArrayUnsafe());
keccak512.update(Longs.toByteArray(Long.reverseBytes(nonce)));
final byte[] seed = keccak512.digest();
final ByteBuffer mixBuffer = ByteBuffer.allocate(MIX_BYTES).order(ByteOrder.LITTLE_ENDIAN);
Expand Down Expand Up @@ -171,7 +172,7 @@ public static void calcDatasetItem(final byte[] buffer, final int[] cache, final
* @param header Block Header
* @return Truncated BlockHeader hash
*/
public static byte[] hashHeader(final SealableBlockHeader header) {
public static Bytes32 hashHeader(final SealableBlockHeader header) {
final BytesValueRLPOutput out = new BytesValueRLPOutput();
out.startList();
out.writeBytes(header.getParentHash());
Expand All @@ -192,7 +193,8 @@ public static byte[] hashHeader(final SealableBlockHeader header) {
out.writeLongScalar(header.getBaseFee().get());
}
out.endList();
return DirectAcyclicGraphSeed.KECCAK_256.get().digest(out.encoded().toArray());
return Bytes32.wrap(
DirectAcyclicGraphSeed.KECCAK_256.get().digest(out.encoded().toArrayUnsafe()));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ public PoWSolution hash(
final long nonce,
final long number,
final EpochCalculator epochCalc,
final byte[] prePowHash) {
final Bytes prePowHash) {

MessageDigest digest = KECCAK_256.get();
digest.update(prePowHash);
digest.update(prePowHash.toArrayUnsafe());
digest.update(Bytes.ofUnsignedLong(nonce).toArrayUnsafe());
Bytes32 solution = Bytes32.wrap(digest.digest());
Hash mixHash = Hash.wrap(solution);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
*/
package org.hyperledger.besu.ethereum.mainnet;

import org.apache.tuweni.bytes.Bytes;

public interface PoWHasher {

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

/** Implementation of Ethash Hashimoto Light Implementation. */
final class EthashLight implements PoWHasher {
Expand All @@ -42,7 +44,7 @@ public PoWSolution hash(
final long nonce,
final long number,
final EpochCalculator epochCalc,
final byte[] prePowHash) {
final Bytes prePowHash) {
final EthHashCacheFactory.EthHashDescriptor cache =
cacheFactory.ethHashCacheFor(number, epochCalc);
final PoWSolution solution =
Expand All @@ -61,7 +63,7 @@ public PoWSolution hash(
final long nonce,
final long number,
final EpochCalculator epochCalc,
final byte[] prePowHash) {
final Bytes prePowHash) {
throw new UnsupportedOperationException("Hashing is unsupported");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,18 @@

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

import java.util.Arrays;
import java.util.Objects;

import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.bytes.Bytes;

public class PoWSolution {
private final long nonce;
private final Hash mixHash;
private final byte[] powHash;
private final Bytes32 solution;
private final Bytes powHash;
private final Bytes solution;

public PoWSolution(
final long nonce, final Hash mixHash, final Bytes32 solution, final byte[] powHash) {
final long nonce, final Hash mixHash, final Bytes solution, final Bytes powHash) {
this.nonce = nonce;
this.mixHash = mixHash;
this.solution = solution;
Expand All @@ -43,11 +42,11 @@ public Hash getMixHash() {
return mixHash;
}

public byte[] getPowHash() {
public Bytes getPowHash() {
return powHash;
}

public Bytes32 getSolution() {
public Bytes getSolution() {
return solution;
}

Expand All @@ -59,13 +58,11 @@ public boolean equals(final Object o) {
return nonce == that.nonce
&& Objects.equals(mixHash, that.mixHash)
&& Objects.equals(solution, that.solution)
&& Arrays.equals(powHash, that.powHash);
&& Objects.equals(powHash, that.powHash);
}

@Override
public int hashCode() {
int result = Objects.hash(nonce, mixHash, solution);
result = 31 * result + Arrays.hashCode(powHash);
return result;
return Objects.hash(nonce, mixHash, solution, powHash);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.hyperledger.besu.ethereum.chain.PoWObserver;
import org.hyperledger.besu.util.Subscribers;

import java.util.Arrays;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -161,7 +160,7 @@ public boolean submitSolution(final PoWSolution solution) {

final PoWSolverJob job = jobSnapshot.get();
final PoWSolverInputs inputs = job.getInputs();
if (!Arrays.equals(inputs.getPrePowHash(), solution.getPowHash())) {
if (!inputs.getPrePowHash().equals(solution.getPowHash())) {
LOG.debug("Miner's solution does not match current job");
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@
*/
package org.hyperledger.besu.ethereum.mainnet;

import java.util.Arrays;

import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.units.bigints.UInt256;

public class PoWSolverInputs {
private final UInt256 target;
private final byte[] prePowHash;
private final Bytes prePowHash;
private final long blockNumber;

public PoWSolverInputs(final UInt256 target, final byte[] prePowHash, final long blockNumber) {
public PoWSolverInputs(final UInt256 target, final Bytes prePowHash, final long blockNumber) {
this.target = target;
this.prePowHash = prePowHash;
this.blockNumber = blockNumber;
Expand All @@ -33,7 +32,7 @@ public UInt256 getTarget() {
return target;
}

public byte[] getPrePowHash() {
public Bytes getPrePowHash() {
return prePowHash;
}

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

@Override
public String toString() {
return "MiningSolverInputs{"
return "PoWSolverInputs{"
+ "target="
+ target
+ ", prePowHash="
+ Arrays.toString(prePowHash)
+ prePowHash
+ ", blockNumber="
+ blockNumber
+ '}';
Expand Down
Loading

0 comments on commit bc70025

Please sign in to comment.