From cbef3e940bc827aa25e4474e8237cc73095d0360 Mon Sep 17 00:00:00 2001 From: SteveM Date: Fri, 29 Nov 2019 21:42:55 -0800 Subject: [PATCH 1/3] eip-2124 RLP encoding checks pass, store next block number as bytesValue Signed-off-by: SteveM --- .../eth/manager/EthProtocolManager.java | 67 +++----- .../ethereum/eth/manager/ForkIdManager.java | 160 ++++++++++++++---- .../ethereum/eth/messages/StatusMessage.java | 27 ++- .../eth/manager/ForkIdManagerTest.java | 118 ++++++++++--- .../eth/messages/StatusMessageTest.java | 4 +- 5 files changed, 267 insertions(+), 109 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index acd9ea8b932..c0eeeb0160c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -286,36 +286,20 @@ public void handleNewConnection(final PeerConnection connection) { final Capability cap = connection.capability(getSupportedProtocol()); // TODO: look to consolidate code below if possible // making status non-final and implementing it above would be one way. - if (cap.getVersion() > EthProtocol.EthVersion.V63) { - final StatusMessage status = - StatusMessage.create( - cap.getVersion(), - networkId, - blockchain.getChainHead().getTotalDifficulty(), - blockchain.getChainHeadHash(), - forkIdManager.getLatestForkId()); - try { - LOG.debug("Sending status message to {}.", peer); - peer.send(status); - peer.registerStatusSent(); - } catch (final PeerNotConnected peerNotConnected) { - // Nothing to do. - } - } else { - final StatusMessage status = - StatusMessage.create( - cap.getVersion(), - networkId, - blockchain.getChainHead().getTotalDifficulty(), - blockchain.getChainHeadHash(), - genesisHash); - try { - LOG.debug("Sending status message to {}.", peer); - peer.send(status); - peer.registerStatusSent(); - } catch (final PeerNotConnected peerNotConnected) { - // Nothing to do. - } + final StatusMessage status = + StatusMessage.create( + cap.getVersion(), + networkId, + blockchain.getChainHead().getTotalDifficulty(), + blockchain.getChainHeadHash(), + genesisHash, + forkIdManager.getLatestForkId()); + try { + LOG.debug("Sending status message to {}.", peer); + peer.send(status); + peer.registerStatusSent(); + } catch (final PeerNotConnected peerNotConnected) { + // Nothing to do. } } @@ -346,18 +330,17 @@ private void handleStatusMessage(final EthPeer peer, final MessageData data) { if (!status.networkId().equals(networkId)) { LOG.debug("Disconnecting from peer with mismatched network id: {}", status.networkId()); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); - } else if (forkIdManager.peerCheck(status)) { - if (status.genesisHash() != null) { - LOG.debug( - "Disconnecting from peer with matching network id ({}), but non-matching genesis hash: {}", - networkId, - status.genesisHash()); - } else { - LOG.debug( - "Disconnecting from peer with matching network id ({}), but non-matching fork id: {}", - networkId, - status.forkId()); - } + } else if (forkIdManager.peerCheck(status.forkId())) { + LOG.debug( + "Disconnecting from peer with matching network id ({}), but non-matching fork id: {}", + networkId, + status.forkId()); + peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); + } else if (forkIdManager.peerCheck(status.genesisHash())) { + LOG.debug( + "Disconnecting from peer with matching network id ({}), but non-matching genesis hash: {}", + networkId, + status.genesisHash()); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); } else { LOG.debug("Received status message from {}: {}", peer, status); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java index 47773188bdf..b10da36c45f 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java @@ -16,7 +16,6 @@ import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Hash; -import org.hyperledger.besu.ethereum.eth.messages.StatusMessage; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput; import org.hyperledger.besu.ethereum.rlp.RLPInput; @@ -33,6 +32,8 @@ import java.util.Set; import java.util.zip.CRC32; +import static java.nio.charset.StandardCharsets.UTF_8; + public class ForkIdManager { private Hash genesisHash; @@ -40,7 +41,7 @@ public class ForkIdManager { private Long currentHead; private Long forkNext; private Long highestKnownFork = 0L; - private ForkId lastKnownEntry; + private ForkId lastKnownEntry = null; private ArrayDeque forkAndHashList; public ForkIdManager(final Hash genesisHash, final Set forks, final Long currentHead) { @@ -82,14 +83,71 @@ public static ForkIdManager buildCollection(final Hash genesisHash) { public static ForkId readFrom(final RLPInput in) { in.enterList(); final BytesValue hash = in.readBytesValue(); - final long next = in.readLong(); + final BytesValue next = in.readBytesValue(); +// final long next = in.readLong(); in.leaveList(); return new ForkId(hash, next); } +// public static ForkId readFrom(final RLPInput in) { +// in.enterList(); +// final BytesValue hash = in.readBytesValue(); +// final long next = in.readLong(); +// in.leaveList(); +// return new ForkId(hash, next); +// } + // Non-RLP entry (for tests) public static ForkId createIdEntry(final String hash, final long next) { - return new ForkId(hash, next); + return new ForkId(BytesValue.wrap(hexStringToByteArray(hash)), BytesValue.wrap(longToBigEndian(next))); + } + + public static ForkId createIdEntry(final String hash, final String next) { + BytesValue bHash; + System.out.print("String hash: "); // todo remove dev item + System.out.print(hash); // todo remove dev item + System.out.print(", String next: "); // todo remove dev item + System.out.println(next); // todo remove dev item + bHash = BytesValue.wrap(hexStringToByteArray(hash)); + if(bHash.size() < 4){ + bHash = padToEightBytes(bHash); + } + if(next.equals("") || next.equals("0x")){ + System.out.println("1"); // todo remove dev item + return new ForkId(bHash, BytesValue.wrap(hexStringToByteArray("0x"))); + } else if (next.startsWith("0x")) { + System.out.println("2"); // todo remove dev item + long asLong = Long.parseLong(next.replaceFirst("0x", ""), 16); + return new ForkId(bHash, BytesValues.trimLeadingZeros(BytesValue.wrap(longToBigEndian(asLong)))); + } else { + System.out.println("3"); // todo remove dev item + return new ForkId(bHash, BytesValue.wrap(longToBigEndian(Long.parseLong(next)))); + } + } + + public static ForkId createIdEntry(final BytesValue hash, final String next) { + if (next.startsWith("0x")) { + String temp = next.replaceFirst("0x", ""); + BytesValue temp2 = BytesValue.wrap(longToBigEndian(Long.parseLong(temp, 16))); + return new ForkId(hash, BytesValues.trimLeadingZeros(temp2)); + } else if(next.equals("")){ + return new ForkId(hash, BytesValue.wrap(hexStringToByteArray("0x"))); + } else { + return new ForkId(hash, BytesValue.wrap(longToBigEndian(Long.parseLong(next)))); + } + } + + public static ForkId createIdEntry(final BytesValue hash, final long next) { + return new ForkId(hash, BytesValue.wrap(longToBigEndian(next))); + } + + private static BytesValue padToEightBytes(final BytesValue hash){ + if(hash.size() < 4){ + BytesValue padded = BytesValues.concatenate(hash, BytesValue.wrap(hexStringToByteArray("0x00"))); + return padToEightBytes(padded); + } else { + return hash; + } } public ArrayDeque getForkAndHashList() { @@ -104,17 +162,14 @@ public boolean forkIdCapable() { return forkAndHashList.size() != 0; } - public boolean peerCheck(final StatusMessage status) { - if (status.forkId() == null && status.genesisHash() != null) { - return peerCheck(status.genesisHash()); - } else if (status.forkId() != null) { - return peerCheck(status.forkId()); - } else { - return false; - } - } - + /** + * EIP-2124 behaviour + * + * @param forkId to be validated. + * @return boolean + */ public boolean peerCheck(final ForkId forkId) { + System.out.println(getForkAndHashList()); // todo remove dev item if (forkId == null) { return false; } @@ -138,8 +193,8 @@ public boolean peerCheck(final ForkId forkId) { if (currentHead < forkNext) { return true; } else { - if (isForkKnown(forkId.next)) { - return isRemoteAwareOfPresent(forkId.hash, forkId.next); + if (isForkKnown(forkId.getNextAsLong())) { + return isRemoteAwareOfPresent(forkId.hash, forkId.getNextAsLong()); } else { return false; } @@ -160,6 +215,7 @@ public boolean peerCheck(final Bytes32 peerGenesisOrCheckSumHash) { } private boolean isHashKnown(final BytesValue forkHash) { + System.out.println(forkHash); // todo remove dev item for (ForkId j : forkAndHashList) { if (forkHash.equals(j.hash)) { return true; @@ -173,7 +229,7 @@ private boolean isForkKnown(final Long nextFork) { return true; } for (ForkId j : forkAndHashList) { - if (nextFork.equals(j.next)) { + if (nextFork.equals(j.getNextAsLong())) { return true; } } @@ -183,9 +239,9 @@ private boolean isForkKnown(final Long nextFork) { private boolean isRemoteAwareOfPresent(final BytesValue forkHash, final Long nextFork) { for (ForkId j : forkAndHashList) { if (forkHash.equals(j.hash)) { - if (nextFork.equals(j.next)) { + if (nextFork.equals(j.getNextAsLong())) { return true; - } else if (j.next == 0L) { + } else if (j.getNextAsLong() == 0L) { return highestKnownFork <= nextFork; // Remote aware of future fork } else { return false; @@ -194,9 +250,10 @@ private boolean isRemoteAwareOfPresent(final BytesValue forkHash, final Long nex } return false; } - +// TODO: sort these when the list of forks is first gathered private ArrayDeque collectForksAndHashes(final Set forks, final Long currentHead) { boolean first = true; + System.out.println(forks); // todo remove dev item ArrayDeque forkList = new ArrayDeque<>(); Iterator iterator = forks.iterator(); while (iterator.hasNext()) { @@ -244,10 +301,6 @@ private BytesValue updateCrc(final String hash) { } public BytesValue getCurrentCrcHash() { - // return "0x" + encodeHexString(BytesValues.ofUnsignedInt(crc.getValue()).getByteArray()); - // System.out.println("0x" + - // encodeHexString(BytesValues.ofUnsignedInt(crc.getValue()).getByteArray())); // todo remove - // dev item return BytesValues.ofUnsignedInt(crc.getValue()); } @@ -255,34 +308,78 @@ public BytesValue getCurrentCrcHash() { // ^ the crc is not hashed/checksum-ed any further so the hash class is not suited for this case public static class ForkId { BytesValue hash; - long next; + BytesValue next; BytesValue forkIdRLP; + public ForkId(final BytesValue hash, final BytesValue next) { + this.hash = hash; + this.next = next; + createForkIdRLP(); + } + + public ForkId(final String hash, final String next) { + this.hash = BytesValue.wrap(hexStringToByteArray(hash)); + if(this.hash.size() < 4){ + this.hash = padToEightBytes(this.hash); + } + if(next.equals("") || next.equals("0x")){ + this.next = BytesValue.wrap(hexStringToByteArray("0x")); + } else if (next.startsWith("0x")) { + long asLong = Long.parseLong(next.replaceFirst("0x", ""), 16); + this.next = BytesValues.trimLeadingZeros(BytesValue.wrap(longToBigEndian(asLong))); + } else { + this.next = BytesValue.wrap(longToBigEndian(Long.parseLong(next))); + } + createForkIdRLP(); + } + public ForkId(final String hash, final long next) { this.hash = BytesValue.wrap(hexStringToByteArray(hash)); - this.next = next; + this.next = BytesValue.wrap(longToBigEndian(next)); + createForkIdRLP(); + } + + public ForkId(final BytesValue hash, final String next) { + this.hash = hash; + this.next = BytesValue.wrap(longToBigEndian(Long.parseLong(next))); createForkIdRLP(); } public ForkId(final BytesValue hash, final long next) { this.hash = hash; - this.next = next; + this.next = BytesValue.wrap(longToBigEndian(next)); createForkIdRLP(); } + public long getNextAsLong(){ + return BytesValues.extractLong(next); + } + + public void createForkIdRLP() { BytesValueRLPOutput out = new BytesValueRLPOutput(); - out.writeList(asList(), ForkId::writeTo); +// out.writeList(asList(), ForkId::writeTo); + writeTo(out); forkIdRLP = out.encoded(); } public void writeTo(final RLPOutput out) { out.startList(); out.writeBytesValue(hash); - out.writeLong(next); + out.writeBytesValue(next); out.endList(); } + // Non-RLP entry (for tests) + public BytesValue createNotAsListForkIdRLP() { + BytesValueRLPOutput outPlain = new BytesValueRLPOutput(); + outPlain.startList(); + outPlain.writeBytesValue(hash); + outPlain.writeBytesValue(next); + outPlain.endList(); + return outPlain.encoded(); + } + public static ForkId readFrom(final RLPInput in) { in.enterList(); final BytesValue hash = in.readBytesValue(); @@ -294,7 +391,7 @@ public static ForkId readFrom(final RLPInput in) { public List asByteList() { ArrayList forRLP = new ArrayList(); forRLP.add(hash.getByteArray()); - forRLP.add(longToBigEndian(next)); + forRLP.add(next.getByteArray()); return forRLP; } @@ -313,7 +410,8 @@ public String toString() { public boolean equals(final Object obj) { if (obj instanceof ForkId) { ForkId other = (ForkId) obj; - return other.hash.equals(this.hash) && other.next == this.next; + return other.hash.equals(this.hash) && other.next.equals(this.next); +// return other.hash.equals(this.hash) && other.next == this.next; } return false; } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessage.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessage.java index c6cf63639bf..75ca7a41cad 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessage.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessage.java @@ -55,9 +55,10 @@ public static StatusMessage create( final BigInteger networkId, final UInt256 totalDifficulty, final Hash bestHash, + final Hash genesisHash, final ForkIdManager.ForkId forkId) { final EthStatus status = - new EthStatus(protocolVersion, networkId, totalDifficulty, bestHash, forkId); + new EthStatus(protocolVersion, networkId, totalDifficulty, bestHash, genesisHash, forkId); final BytesValueRLPOutput out = new BytesValueRLPOutput(); status.writeTo(out); @@ -108,9 +109,7 @@ public Bytes32 genesisHash() { return status().genesisHash; } - /** - * @return The fork id of the network the associated node is participating in. - */ + /** @return The fork id of the network the associated node is participating in. */ public ForkIdManager.ForkId forkId() { return status().forkId; } @@ -150,13 +149,14 @@ private static class EthStatus { final BigInteger networkId, final UInt256 totalDifficulty, final Hash bestHash, + final Hash genesisHash, final ForkIdManager.ForkId forkHash) { this.protocolVersion = protocolVersion; this.networkId = networkId; this.totalDifficulty = totalDifficulty; this.bestHash = bestHash; + this.genesisHash = genesisHash; this.forkId = forkHash; - this.genesisHash = null; } public void writeTo(final RLPOutput out) { @@ -166,10 +166,9 @@ public void writeTo(final RLPOutput out) { out.writeBigIntegerScalar(networkId); out.writeUInt256Scalar(totalDifficulty); out.writeBytesValue(bestHash); + out.writeBytesValue(genesisHash); if (forkId != null) { forkId.writeTo(out); - } else { - out.writeBytesValue(genesisHash); } out.endList(); @@ -182,18 +181,16 @@ public static EthStatus readFrom(final RLPInput in) { final BigInteger networkId = in.readBigIntegerScalar(); final UInt256 totalDifficulty = in.readUInt256Scalar(); final Hash bestHash = Hash.wrap(in.readBytes32()); + final Hash genesisHash = Hash.wrap(in.readBytes32()); if (in.nextIsList()) { final ForkIdManager.ForkId forkId = ForkIdManager.ForkId.readFrom(in); in.leaveList(); - - return new EthStatus(protocolVersion, networkId, totalDifficulty, bestHash, forkId); - } else { - final Hash genesisHash = Hash.wrap(in.readBytes32()); - - in.leaveList(); - - return new EthStatus(protocolVersion, networkId, totalDifficulty, bestHash, genesisHash); + return new EthStatus( + protocolVersion, networkId, totalDifficulty, bestHash, genesisHash, forkId); } + in.leaveList(); + + return new EthStatus(protocolVersion, networkId, totalDifficulty, bestHash, genesisHash); } } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java index 33c42c49823..521533291d7 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java @@ -19,14 +19,19 @@ import org.hyperledger.besu.ethereum.core.Hash; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput; +// import org.hyperledger.besu.ethereum.rlp.RLP; +//import org.hyperledger.besu.ethereum.rlp.RLPException; +// import org.hyperledger.besu.ethereum.rlp.RLPInput; import org.hyperledger.besu.util.bytes.BytesValue; import java.util.ArrayDeque; +// import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +//import org.hyperledger.besu.util.bytes.BytesValues; import org.junit.Test; public class ForkIdManagerTest { @@ -50,9 +55,11 @@ public void checkCorrectMainnetForkIdHashesGenerated() { ForkIdManager.createIdEntry("0x3edd5b10", 4370000L), // First Spurious block ForkIdManager.createIdEntry("0xa00bc324", 7280000L), // First Byzantium block ForkIdManager.createIdEntry("0x668db0af", 0L) // Today Petersburg block + }; List list = Arrays.asList(forksMainnet); - ForkIdManager forkIdManager = ForkIdManager.buildCollection(Hash.fromHexString(mainnetGenHash), list); + ForkIdManager forkIdManager = + ForkIdManager.buildCollection(Hash.fromHexString(mainnetGenHash), list); ArrayDeque entries = forkIdManager.getForkAndHashList(); for (ForkIdManager.ForkId id : checkIds) { ForkIdManager.ForkId testVal = entries.poll(); @@ -121,7 +128,8 @@ public void check1PetersburgWithRemoteAnnouncingTheSame() { // {7987396, ID{Hash: 0x668db0af, Next: 0}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", 0L)); assertThat(result).isTrue(); } @@ -133,8 +141,10 @@ public void check2PetersburgWithRemoteAnnouncingTheSameAndNextFork() { // {7987396, ID{Hash: 0x668db0af, Next: math.MaxUint64}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); - Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", Long.MAX_VALUE)); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + Boolean result = + forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", Long.MAX_VALUE)); assertThat(result).isTrue(); } @@ -147,7 +157,8 @@ public void check3ByzantiumAwareOfPetersburgRemoteUnawareOfPetersburg() { // {7279999, ID{Hash: 0xa00bc324, Next: 0}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 0L)); assertThat(result).isTrue(); } @@ -160,7 +171,8 @@ public void check4ByzantiumAwareOfPetersburgRemoteAwareOfPetersburg() { // {7279999, ID{Hash: 0xa00bc324, Next: 7280000}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 7280000L)); assertThat(result).isTrue(); } @@ -174,8 +186,10 @@ public void check5ByzantiumAwareOfPetersburgRemoteAnnouncingUnknownFork() { // {7279999, ID{Hash: 0xa00bc324, Next: math.MaxUint64}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); - Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", Long.MAX_VALUE)); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + Boolean result = + forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", Long.MAX_VALUE)); assertThat(result).isTrue(); } @@ -186,7 +200,8 @@ public void check6PetersburgWithRemoteAnnouncingByzantiumAwareOfPetersburg() { // {7987396, ID{Hash: 0x668db0af, Next: 7280000}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", 7280000L)); assertThat(result).isTrue(); } @@ -199,7 +214,8 @@ public void check7PetersburgWithRemoteAnnouncingSpuriousAwareOfByzantiumRemoteMa // {7987396, ID{Hash: 0x3edd5b10, Next: 4370000}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x3edd5b10", 4370000L)); assertThat(result).isTrue(); } @@ -210,7 +226,8 @@ public void check8ByzantiumWithRemoteAnnouncingPetersburgLocalOutOfSync() { // {7279999, ID{Hash: 0x668db0af, Next: 0}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", 0L)); assertThat(result).isTrue(); } @@ -222,7 +239,8 @@ public void check9SpuriousWithRemoteAnnouncingByzantiumRemoteUnawareOfPetersburg // {4369999, ID{Hash: 0xa00bc324, Next: 0}, nil}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 4369999L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 4369999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 0L)); assertThat(result).isTrue(); } @@ -234,7 +252,8 @@ public void check10PetersburgWithRemoteAnnouncingByzantiumRemoteUnawareOfAdditio // {7987396, ID{Hash: 0xa00bc324, Next: 0}, ErrRemoteStale}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 0L)); assertThat(result).isFalse(); } @@ -246,7 +265,8 @@ public void check11PetersburgWithRemoteAnnouncingPetersburgAndFutureForkLocalNee // {7987396, ID{Hash: 0x5cddc0e1, Next: 0}, ErrLocalIncompatibleOrStale}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x5cddc0e1", 0L)); assertThat(result).isFalse(); } @@ -258,7 +278,8 @@ public void check12ByzantiumWithRemoteAnnouncingPetersburgAndFutureForkLocalNeed // {7279999, ID{Hash: 0x5cddc0e1, Next: 0}, ErrLocalIncompatibleOrStale}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x5cddc0e1", 0L)); assertThat(result).isFalse(); } @@ -269,7 +290,8 @@ public void check13ByzantiumWithRemoteAnnouncingRinkebyPetersburg() { // {7987396, ID{Hash: 0xafec6b27, Next: 0}, ErrLocalIncompatibleOrStale}, List list = Arrays.asList(forksMainnet); Set forkSet = new LinkedHashSet<>(list); - ForkIdManager forkIdManager = new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + ForkIdManager forkIdManager = + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xafec6b27", 0L)); assertThat(result).isFalse(); } @@ -278,11 +300,67 @@ public void check13ByzantiumWithRemoteAnnouncingRinkebyPetersburg() { public void createAndDecodeRLP() { ForkIdManager.ForkId forkIdEntry = ForkIdManager.createIdEntry("0xa00bc324", 7280000L); BytesValueRLPOutput out = new BytesValueRLPOutput(); - out.writeList(forkIdEntry.asList(), ForkIdManager.ForkId::writeTo); + forkIdEntry.writeTo(out); + BytesValue bytesValue = out.encoded(); + BytesValueRLPInput in = new BytesValueRLPInput(bytesValue, false); + ForkIdManager.ForkId decodedEntry = ForkIdManager.readFrom(in); + assertThat(forkIdEntry.equals(decodedEntry)).isTrue(); + } + + // OLD VERSION + // @Test + // public void createAndDecodeRLP() { + // ForkIdManager.ForkId forkIdEntry = ForkIdManager.createIdEntry("0xa00bc324", 7280000L); + // BytesValueRLPOutput out = new BytesValueRLPOutput(); + // out.writeList(forkIdEntry.asList(), ForkIdManager.ForkId::writeTo); + // BytesValue bytesValue = out.encoded(); + // BytesValueRLPInput in = new BytesValueRLPInput(bytesValue, false); + // List forkId = in.readList(ForkIdManager::readFrom); + // ForkIdManager.ForkId decodedEntry = forkId.get(0); + // assertThat(forkIdEntry.equals(decodedEntry)).isTrue(); + // } + + @Test + public void check1ZeroZeroProperRLPEncoding() { + ForkIdManager.ForkId forkIdEntry = ForkIdManager.createIdEntry("0", "0x"); + System.out.println(forkIdEntry); // todo remove dev item + BytesValueRLPOutput out = new BytesValueRLPOutput(); + forkIdEntry.writeTo(out); + String str1 = "0xc6840000000080"; + BytesValue bytesValue = out.encoded(); + System.out.println(bytesValue); // todo remove dev item + assertThat(str1.equals(bytesValue.toString())).isTrue(); + BytesValueRLPInput in = new BytesValueRLPInput(bytesValue, false); + ForkIdManager.ForkId decodedEntry = ForkIdManager.readFrom(in); + assertThat(forkIdEntry.equals(decodedEntry)).isTrue(); + } + + @Test + public void check2ArbitraryProperRLPEncoding() { + ForkIdManager.ForkId forkIdEntry = ForkIdManager.createIdEntry("0xdeadbeef", "0xBADDCAFE"); + BytesValueRLPOutput out = new BytesValueRLPOutput(); + forkIdEntry.writeTo(out); + String str1 = "0xca84deadbeef84baddcafe"; + BytesValue bytesValue = out.encoded(); + assertThat(str1.equals(bytesValue.toString())).isTrue(); + BytesValueRLPInput in = new BytesValueRLPInput(bytesValue, false); + ForkIdManager.ForkId decodedEntry = ForkIdManager.readFrom(in); + assertThat(forkIdEntry.equals(decodedEntry)).isTrue(); + } + + @Test + public void check3MaximumsProperRLPEncoding() { + ForkIdManager.ForkId forkIdEntry = ForkIdManager.createIdEntry("0xffffffff", Long.MAX_VALUE); + BytesValueRLPOutput out = new BytesValueRLPOutput(); + forkIdEntry.writeTo(out); + // ce84ffffffff88ffffffffffffffff; // Check value supplied in EIP-2124 spec via GO lang + // math.MaxUint64 + String str1 = + "0xce84ffffffff887fffffffffffffff"; // Long.MAX_VALUE is smaller than GO lang math.MaxUint64 BytesValue bytesValue = out.encoded(); + assertThat(str1.equals(bytesValue.toString())).isTrue(); BytesValueRLPInput in = new BytesValueRLPInput(bytesValue, false); - List forkId = in.readList(ForkIdManager::readFrom); - ForkIdManager.ForkId decodedEntry = forkId.get(0); + ForkIdManager.ForkId decodedEntry = ForkIdManager.readFrom(in); assertThat(forkIdEntry.equals(decodedEntry)).isTrue(); } -} + } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessageTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessageTest.java index 93b24722c64..1e5ccbfd430 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessageTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessageTest.java @@ -73,9 +73,10 @@ public void serializeDeserializeWithForkId() { final BigInteger networkId = BigInteger.ONE; final UInt256 td = UInt256.of(1000L); final Hash bestHash = randHash(1L); + final Hash genesisHash = randHash(2L); final ForkIdManager.ForkId forkId = ForkIdManager.createIdEntry("0xa00bc334", 0L); - final MessageData msg = StatusMessage.create(version, networkId, td, bestHash, forkId); + final MessageData msg = StatusMessage.create(version, networkId, td, bestHash, genesisHash, forkId); final StatusMessage copy = new StatusMessage(msg.getData()); @@ -83,6 +84,7 @@ public void serializeDeserializeWithForkId() { assertThat(copy.networkId()).isEqualTo(networkId); assertThat(copy.totalDifficulty()).isEqualTo(td); assertThat(copy.bestHash()).isEqualTo(bestHash); + assertThat(copy.genesisHash()).isEqualTo(genesisHash); assertThat(copy.forkId()).isEqualTo(forkId); } From 9c946a8aeedcd5f85d5e6db167ef71c10710fc8a Mon Sep 17 00:00:00 2001 From: SteveM Date: Sat, 30 Nov 2019 00:19:45 -0800 Subject: [PATCH 2/3] eip-2124 RLP encoding checks pass, store next block number as bytesValue, include backward compatibility of eip-2364 Signed-off-by: SteveM --- .../besu/ethereum/eth/EthProtocol.java | 1 + .../eth/manager/EthProtocolManager.java | 4 +- .../ethereum/eth/manager/ForkIdManager.java | 176 ++++++------------ .../eth/manager/ForkIdManagerTest.java | 15 -- 4 files changed, 64 insertions(+), 132 deletions(-) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java index 6d0e77409fc..775aafca359 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java @@ -31,6 +31,7 @@ public class EthProtocol implements SubProtocol { public static final String NAME = "eth"; public static final Capability ETH62 = Capability.create(NAME, EthVersion.V62); public static final Capability ETH63 = Capability.create(NAME, EthVersion.V63); + public static final Capability ETH64 = Capability.create(NAME, EthVersion.V64); private static final EthProtocol INSTANCE = new EthProtocol(); private static final List eth62Messages = diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index c0eeeb0160c..610abc618c8 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -179,7 +179,7 @@ public EthProtocolManager( genesisHash = blockchain.getBlockHashByNumber(0L).get(); forkIdManager = - ForkIdManager.buildCollection(genesisHash, forks, blockchain, getSupportedCapabilities()); + ForkIdManager.buildCollection(genesisHash, forks, blockchain); ethPeers = new EthPeers(getSupportedProtocol(), clock, metricsSystem); ethMessages = new EthMessages(); @@ -330,7 +330,7 @@ private void handleStatusMessage(final EthPeer peer, final MessageData data) { if (!status.networkId().equals(networkId)) { LOG.debug("Disconnecting from peer with mismatched network id: {}", status.networkId()); peer.disconnect(DisconnectReason.SUBPROTOCOL_TRIGGERED); - } else if (forkIdManager.peerCheck(status.forkId())) { + } else if (!forkIdManager.peerCheck(status.forkId()) && status.protocolVersion() > 63) { LOG.debug( "Disconnecting from peer with matching network id ({}), but non-matching fork id: {}", networkId, diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java index b10da36c45f..7e93a9addf4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java @@ -16,7 +16,6 @@ import org.hyperledger.besu.ethereum.chain.Blockchain; import org.hyperledger.besu.ethereum.core.Hash; -import org.hyperledger.besu.ethereum.p2p.rlpx.wire.Capability; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput; import org.hyperledger.besu.ethereum.rlp.RLPInput; import org.hyperledger.besu.ethereum.rlp.RLPOutput; @@ -32,8 +31,6 @@ import java.util.Set; import java.util.zip.CRC32; -import static java.nio.charset.StandardCharsets.UTF_8; - public class ForkIdManager { private Hash genesisHash; @@ -54,11 +51,8 @@ public ForkIdManager(final Hash genesisHash, final Set forks, final Long c } }; - public static ForkIdManager buildCollection( - final Hash genesisHash, - final List forks, - final Blockchain blockchain, - final List caps) { + static ForkIdManager buildCollection( + final Hash genesisHash, final List forks, final Blockchain blockchain) { if (forks == null) { return new ForkIdManager(genesisHash, null, blockchain.getChainHeadBlockNumber()); } else { @@ -76,102 +70,47 @@ public static ForkIdManager buildCollection(final Hash genesisHash, final List getForkAndHashList() { return this.forkAndHashList; } - public ForkId getLatestForkId() { + ForkId getLatestForkId() { return lastKnownEntry; } - public boolean forkIdCapable() { - return forkAndHashList.size() != 0; + public static ForkId readFrom(final RLPInput in) { + in.enterList(); + final BytesValue hash = in.readBytesValue(); + final BytesValue next = in.readBytesValue(); + in.leaveList(); + return new ForkId(hash, next); } + + /** * EIP-2124 behaviour * * @param forkId to be validated. - * @return boolean + * @return boolean (peer valid (true) or invalid (false)) */ public boolean peerCheck(final ForkId forkId) { - System.out.println(getForkAndHashList()); // todo remove dev item if (forkId == null) { - return false; + return true; // Another method must be used to validate (i.e. genesis hash) } // Run the fork checksum validation ruleset: // 1. If local and remote FORK_CSUM matches, connect. @@ -189,12 +128,12 @@ public boolean peerCheck(final ForkId forkId) { // the remote, but at this current point in time we don't have enough // information. // 4. Reject in all other cases. - if (isHashKnown(forkId.hash)) { + if (isHashKnown(forkId.getHash())) { if (currentHead < forkNext) { return true; } else { - if (isForkKnown(forkId.getNextAsLong())) { - return isRemoteAwareOfPresent(forkId.hash, forkId.getNextAsLong()); + if (isForkKnown(forkId.getNext())) { + return isRemoteAwareOfPresent(forkId.getHash(), forkId.getNext()); } else { return false; } @@ -215,9 +154,8 @@ public boolean peerCheck(final Bytes32 peerGenesisOrCheckSumHash) { } private boolean isHashKnown(final BytesValue forkHash) { - System.out.println(forkHash); // todo remove dev item for (ForkId j : forkAndHashList) { - if (forkHash.equals(j.hash)) { + if (forkHash.equals(j.getHash())) { return true; } } @@ -229,7 +167,7 @@ private boolean isForkKnown(final Long nextFork) { return true; } for (ForkId j : forkAndHashList) { - if (nextFork.equals(j.getNextAsLong())) { + if (nextFork.equals(j.getNext())) { return true; } } @@ -238,11 +176,11 @@ private boolean isForkKnown(final Long nextFork) { private boolean isRemoteAwareOfPresent(final BytesValue forkHash, final Long nextFork) { for (ForkId j : forkAndHashList) { - if (forkHash.equals(j.hash)) { - if (nextFork.equals(j.getNextAsLong())) { + if (forkHash.equals(j.getHash())) { + if (nextFork.equals(j.getNext())) { return true; - } else if (j.getNextAsLong() == 0L) { - return highestKnownFork <= nextFork; // Remote aware of future fork + } else if (j.getNext() == 0L) { + return highestKnownFork <= nextFork; // Remote aware of an additional future fork } else { return false; } @@ -250,10 +188,10 @@ private boolean isRemoteAwareOfPresent(final BytesValue forkHash, final Long nex } return false; } -// TODO: sort these when the list of forks is first gathered + + // TODO: sort these when the list of forks is first gathered private ArrayDeque collectForksAndHashes(final Set forks, final Long currentHead) { boolean first = true; - System.out.println(forks); // todo remove dev item ArrayDeque forkList = new ArrayDeque<>(); Iterator iterator = forks.iterator(); while (iterator.hasNext()) { @@ -272,11 +210,14 @@ private ArrayDeque collectForksAndHashes(final Set forks, final Lo // most recent fork forkList.add(new ForkId(getCurrentCrcHash(), forkBlockNumber)); updateCrc(forkBlockNumber); - lastKnownEntry = new ForkId(getCurrentCrcHash(), 0L); - forkList.add(lastKnownEntry); + // next fork or no future fork known if (currentHead > forkBlockNumber) { + lastKnownEntry = new ForkId(getCurrentCrcHash(), 0L); + forkList.add(lastKnownEntry); this.forkNext = 0L; } else { + lastKnownEntry = new ForkId(getCurrentCrcHash(), forkBlockNumber); + forkList.add(lastKnownEntry); this.forkNext = forkBlockNumber; } @@ -288,10 +229,9 @@ private ArrayDeque collectForksAndHashes(final Set forks, final Lo return forkList; } - private BytesValue updateCrc(final Long block) { + private void updateCrc(final Long block) { byte[] byteRepresentationFork = longToBigEndian(block); crc.update(byteRepresentationFork, 0, byteRepresentationFork.length); - return getCurrentCrcHash(); } private BytesValue updateCrc(final String hash) { @@ -300,7 +240,7 @@ private BytesValue updateCrc(final String hash) { return getCurrentCrcHash(); } - public BytesValue getCurrentCrcHash() { + private BytesValue getCurrentCrcHash() { return BytesValues.ofUnsignedInt(crc.getValue()); } @@ -311,54 +251,50 @@ public static class ForkId { BytesValue next; BytesValue forkIdRLP; - public ForkId(final BytesValue hash, final BytesValue next) { + ForkId(final BytesValue hash, final BytesValue next) { this.hash = hash; this.next = next; createForkIdRLP(); } - public ForkId(final String hash, final String next) { + ForkId(final String hash, final String next) { this.hash = BytesValue.wrap(hexStringToByteArray(hash)); - if(this.hash.size() < 4){ + if (this.hash.size() < 4) { this.hash = padToEightBytes(this.hash); } - if(next.equals("") || next.equals("0x")){ + if (next.equals("") || next.equals("0x")) { this.next = BytesValue.wrap(hexStringToByteArray("0x")); } else if (next.startsWith("0x")) { long asLong = Long.parseLong(next.replaceFirst("0x", ""), 16); this.next = BytesValues.trimLeadingZeros(BytesValue.wrap(longToBigEndian(asLong))); - } else { + } else { this.next = BytesValue.wrap(longToBigEndian(Long.parseLong(next))); } createForkIdRLP(); } - public ForkId(final String hash, final long next) { + ForkId(final String hash, final long next) { this.hash = BytesValue.wrap(hexStringToByteArray(hash)); this.next = BytesValue.wrap(longToBigEndian(next)); createForkIdRLP(); } - public ForkId(final BytesValue hash, final String next) { - this.hash = hash; - this.next = BytesValue.wrap(longToBigEndian(Long.parseLong(next))); - createForkIdRLP(); - } - - public ForkId(final BytesValue hash, final long next) { + ForkId(final BytesValue hash, final long next) { this.hash = hash; this.next = BytesValue.wrap(longToBigEndian(next)); createForkIdRLP(); } - public long getNextAsLong(){ + public long getNext() { return BytesValues.extractLong(next); } + public BytesValue getHash() { + return hash; + } - public void createForkIdRLP() { + void createForkIdRLP() { BytesValueRLPOutput out = new BytesValueRLPOutput(); -// out.writeList(asList(), ForkId::writeTo); writeTo(out); forkIdRLP = out.encoded(); } @@ -401,17 +337,27 @@ public List asList() { return forRLP; } + private static BytesValue padToEightBytes(final BytesValue hash) { + if (hash.size() < 4) { + BytesValue padded = + BytesValues.concatenate(hash, BytesValue.wrap(hexStringToByteArray("0x00"))); + return padToEightBytes(padded); + } else { + return hash; + } + } + @Override public String toString() { - return "ForkId(hash=" + this.hash + ", next=" + this.next + ")"; + return "ForkId(hash=" + this.hash + ", next=" + BytesValues.extractLong(next) + ")"; } @Override public boolean equals(final Object obj) { if (obj instanceof ForkId) { ForkId other = (ForkId) obj; - return other.hash.equals(this.hash) && other.next.equals(this.next); -// return other.hash.equals(this.hash) && other.next == this.next; + Long thisNext = BytesValues.extractLong(next); + return other.getHash().equals(this.hash) && thisNext.equals(other.getNext()); } return false; } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java index 521533291d7..ebd695fa3d2 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java @@ -307,28 +307,13 @@ public void createAndDecodeRLP() { assertThat(forkIdEntry.equals(decodedEntry)).isTrue(); } - // OLD VERSION - // @Test - // public void createAndDecodeRLP() { - // ForkIdManager.ForkId forkIdEntry = ForkIdManager.createIdEntry("0xa00bc324", 7280000L); - // BytesValueRLPOutput out = new BytesValueRLPOutput(); - // out.writeList(forkIdEntry.asList(), ForkIdManager.ForkId::writeTo); - // BytesValue bytesValue = out.encoded(); - // BytesValueRLPInput in = new BytesValueRLPInput(bytesValue, false); - // List forkId = in.readList(ForkIdManager::readFrom); - // ForkIdManager.ForkId decodedEntry = forkId.get(0); - // assertThat(forkIdEntry.equals(decodedEntry)).isTrue(); - // } - @Test public void check1ZeroZeroProperRLPEncoding() { ForkIdManager.ForkId forkIdEntry = ForkIdManager.createIdEntry("0", "0x"); - System.out.println(forkIdEntry); // todo remove dev item BytesValueRLPOutput out = new BytesValueRLPOutput(); forkIdEntry.writeTo(out); String str1 = "0xc6840000000080"; BytesValue bytesValue = out.encoded(); - System.out.println(bytesValue); // todo remove dev item assertThat(str1.equals(bytesValue.toString())).isTrue(); BytesValueRLPInput in = new BytesValueRLPInput(bytesValue, false); ForkIdManager.ForkId decodedEntry = ForkIdManager.readFrom(in); From 35f012f7ace33d62aec1f2c0b221558a5b9f2cec Mon Sep 17 00:00:00 2001 From: SteveM Date: Sat, 30 Nov 2019 01:13:03 -0800 Subject: [PATCH 3/3] remove redundant methods in forkId and modify fork collection to use a list throughout Signed-off-by: SteveM --- .../besu/config/GenesisConfigFile.java | 22 +++--- .../ethereum/eth/manager/ForkIdManager.java | 73 ++++--------------- .../eth/manager/ForkIdManagerTest.java | 72 +++++++----------- 3 files changed, 50 insertions(+), 117 deletions(-) diff --git a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java index 089b850385d..7149e8e357b 100644 --- a/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java +++ b/config/src/main/java/org/hyperledger/besu/config/GenesisConfigFile.java @@ -16,19 +16,15 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.stream.Collectors.toList; -import static java.util.stream.Collectors.toSet; import static org.hyperledger.besu.config.JsonUtil.normalizeKeys; import java.io.IOException; -import java.util.ArrayList; import java.util.Collections; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.stream.Stream; -import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.Streams; import com.google.common.io.Resources; @@ -137,17 +133,17 @@ public long getTimestamp() { return parseLong("timestamp", JsonUtil.getValueAsString(configRoot, "timestamp", "0x0")); } - // TODO look into how to handle invalid or exceptional (i.e. null or "") fork values public List getForks() { return JsonUtil.getObjectNode(configRoot, "config").stream() - .flatMap(node -> - Streams.stream(node.fieldNames()) - .filter(name -> !name.toLowerCase().equals("chainid")) - .filter(name -> node.get(name).canConvertToLong()) - .filter(name -> name.contains("block")) - .map(name -> node.get(name).asLong()) - ) - .collect(toList()); + .flatMap( + node -> + Streams.stream(node.fieldNames()) + .filter(name -> !name.toLowerCase().equals("chainid")) + .filter(name -> node.get(name).canConvertToLong()) + .filter(name -> name.contains("block")) + .map(name -> node.get(name).asLong())) + .distinct() + .collect(toList()); } private String getRequiredString(final String key) { diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java index 7e93a9addf4..8ad8341aa0c 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java @@ -26,9 +26,7 @@ import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Set; import java.util.zip.CRC32; public class ForkIdManager { @@ -41,7 +39,7 @@ public class ForkIdManager { private ForkId lastKnownEntry = null; private ArrayDeque forkAndHashList; - public ForkIdManager(final Hash genesisHash, final Set forks, final Long currentHead) { + public ForkIdManager(final Hash genesisHash, final List forks, final Long currentHead) { this.genesisHash = genesisHash; this.currentHead = currentHead; if (forks != null) { @@ -56,8 +54,7 @@ static ForkIdManager buildCollection( if (forks == null) { return new ForkIdManager(genesisHash, null, blockchain.getChainHeadBlockNumber()); } else { - Set forkSet = new LinkedHashSet<>(forks); - return new ForkIdManager(genesisHash, forkSet, blockchain.getChainHeadBlockNumber()); + return new ForkIdManager(genesisHash, forks, blockchain.getChainHeadBlockNumber()); } }; @@ -65,8 +62,7 @@ public static ForkIdManager buildCollection(final Hash genesisHash, final List forkSet = new LinkedHashSet<>(forks); - return new ForkIdManager(genesisHash, forkSet, Long.MAX_VALUE); + return new ForkIdManager(genesisHash, forks, Long.MAX_VALUE); } }; @@ -100,8 +96,6 @@ public static ForkId readFrom(final RLPInput in) { return new ForkId(hash, next); } - - /** * EIP-2124 behaviour * @@ -146,11 +140,11 @@ public boolean peerCheck(final ForkId forkId) { /** * Non EIP-2124 behaviour * - * @param peerGenesisOrCheckSumHash Hash or checksum to be validated. + * @param peerGenesisHash Hash to be validated. * @return boolean */ - public boolean peerCheck(final Bytes32 peerGenesisOrCheckSumHash) { - return !peerGenesisOrCheckSumHash.equals(genesisHash); + public boolean peerCheck(final Bytes32 peerGenesisHash) { + return !peerGenesisHash.equals(genesisHash); } private boolean isHashKnown(final BytesValue forkHash) { @@ -189,8 +183,8 @@ private boolean isRemoteAwareOfPresent(final BytesValue forkHash, final Long nex return false; } - // TODO: sort these when the list of forks is first gathered - private ArrayDeque collectForksAndHashes(final Set forks, final Long currentHead) { + // TODO: should sort these when first gathering the list of forks to ensure order + private ArrayDeque collectForksAndHashes(final List forks, final Long currentHead) { boolean first = true; ArrayDeque forkList = new ArrayDeque<>(); Iterator iterator = forks.iterator(); @@ -235,7 +229,8 @@ private void updateCrc(final Long block) { } private BytesValue updateCrc(final String hash) { - byte[] byteRepresentation = hexStringToByteArray(hash); + BytesValue bv = BytesValue.fromHexString(hash); + byte[] byteRepresentation = bv.extractArray(); crc.update(byteRepresentation, 0, byteRepresentation.length); return getCurrentCrcHash(); } @@ -244,8 +239,6 @@ private BytesValue getCurrentCrcHash() { return BytesValues.ofUnsignedInt(crc.getValue()); } - // TODO use Hash class instead of string for checksum. convert to or from string only when needed - // ^ the crc is not hashed/checksum-ed any further so the hash class is not suited for this case public static class ForkId { BytesValue hash; BytesValue next; @@ -258,12 +251,12 @@ public static class ForkId { } ForkId(final String hash, final String next) { - this.hash = BytesValue.wrap(hexStringToByteArray(hash)); + this.hash = BytesValue.fromHexString((hash.length() % 2 == 0 ? "" : "0") + hash); if (this.hash.size() < 4) { this.hash = padToEightBytes(this.hash); } if (next.equals("") || next.equals("0x")) { - this.next = BytesValue.wrap(hexStringToByteArray("0x")); + this.next = BytesValue.EMPTY; } else if (next.startsWith("0x")) { long asLong = Long.parseLong(next.replaceFirst("0x", ""), 16); this.next = BytesValues.trimLeadingZeros(BytesValue.wrap(longToBigEndian(asLong))); @@ -274,7 +267,7 @@ public static class ForkId { } ForkId(final String hash, final long next) { - this.hash = BytesValue.wrap(hexStringToByteArray(hash)); + this.hash = BytesValue.fromHexString(hash); this.next = BytesValue.wrap(longToBigEndian(next)); createForkIdRLP(); } @@ -340,7 +333,7 @@ public List asList() { private static BytesValue padToEightBytes(final BytesValue hash) { if (hash.size() < 4) { BytesValue padded = - BytesValues.concatenate(hash, BytesValue.wrap(hexStringToByteArray("0x00"))); + BytesValues.concatenate(hash, BytesValue.fromHexString("0x00")); return padToEightBytes(padded); } else { return hash; @@ -368,17 +361,7 @@ public int hashCode() { } } - // TODO: Ask / look to see if there is a helper for these below <---------- - private static byte[] hexStringToByteArray(final String s) { - String string = ""; - if (s.startsWith("0x")) { - string = s.replaceFirst("0x", ""); - } - string = (string.length() % 2 == 0 ? "" : "0") + string; - return decodeHexString(string); - } - - // next three methods adopted from: + // next two methods adopted from: // https://github.com/bcgit/bc-java/blob/master/core/src/main/java/org/bouncycastle/util/Pack.java private static byte[] longToBigEndian(final long n) { byte[] bs = new byte[8]; @@ -394,30 +377,4 @@ private static void intToBigEndian(final int n, final byte[] bs, int off) { bs[++off] = (byte) (n >>> 8); bs[++off] = (byte) (n); } - - private static byte[] decodeHexString(final String hexString) { - if (hexString.length() % 2 == 1) { - throw new IllegalArgumentException("Invalid hexadecimal String supplied."); - } - - byte[] bytes = new byte[hexString.length() / 2]; - for (int i = 0; i < hexString.length(); i += 2) { - bytes[i / 2] = hexToByte(hexString.substring(i, i + 2)); - } - return bytes; - } - - private static byte hexToByte(final String hexString) { - int firstDigit = toDigit(hexString.charAt(0)); - int secondDigit = toDigit(hexString.charAt(1)); - return (byte) ((firstDigit << 4) + secondDigit); - } - - private static int toDigit(final char hexChar) { - int digit = Character.digit(hexChar, 16); - if (digit == -1) { - throw new IllegalArgumentException("Invalid Hexadecimal Character: " + hexChar); - } - return digit; - } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java index ebd695fa3d2..4d5ebeb5eb6 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManagerTest.java @@ -19,19 +19,12 @@ import org.hyperledger.besu.ethereum.core.Hash; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput; -// import org.hyperledger.besu.ethereum.rlp.RLP; -//import org.hyperledger.besu.ethereum.rlp.RLPException; -// import org.hyperledger.besu.ethereum.rlp.RLPInput; import org.hyperledger.besu.util.bytes.BytesValue; import java.util.ArrayDeque; -// import java.util.ArrayList; import java.util.Arrays; -import java.util.LinkedHashSet; import java.util.List; -import java.util.Set; -//import org.hyperledger.besu.util.bytes.BytesValues; import org.junit.Test; public class ForkIdManagerTest { @@ -126,10 +119,9 @@ public void checkCorrectRinkebyForkIdHashesGenerated() { public void check1PetersburgWithRemoteAnnouncingTheSame() { // 1 Local is mainnet Petersburg, remote announces the same. No future fork is announced. // {7987396, ID{Hash: 0x668db0af, Next: 0}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", 0L)); assertThat(result).isTrue(); } @@ -139,10 +131,9 @@ public void check2PetersburgWithRemoteAnnouncingTheSameAndNextFork() { // 2 Local is mainnet Petersburg, remote announces the same. Remote also announces a next fork // at block 0xffffffff, but that is uncertain. // {7987396, ID{Hash: 0x668db0af, Next: math.MaxUint64}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", Long.MAX_VALUE)); assertThat(result).isTrue(); @@ -155,10 +146,9 @@ public void check3ByzantiumAwareOfPetersburgRemoteUnawareOfPetersburg() { // the fork). // In this case we don't know if Petersburg passed yet or not. // {7279999, ID{Hash: 0xa00bc324, Next: 0}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 0L)); assertThat(result).isTrue(); } @@ -169,10 +159,9 @@ public void check4ByzantiumAwareOfPetersburgRemoteAwareOfPetersburg() { // announces also Byzantium, and it's also aware of Petersburg (e.g. updated node before the // fork). We don't know if Petersburg passed yet (will pass) or not. // {7279999, ID{Hash: 0xa00bc324, Next: 7280000}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 7280000L)); assertThat(result).isTrue(); } @@ -184,10 +173,9 @@ public void check5ByzantiumAwareOfPetersburgRemoteAnnouncingUnknownFork() { // Petersburg). // As neither forks passed at neither nodes, they may mismatch, but we still connect for now. // {7279999, ID{Hash: 0xa00bc324, Next: math.MaxUint64}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", Long.MAX_VALUE)); assertThat(result).isTrue(); @@ -198,10 +186,9 @@ public void check6PetersburgWithRemoteAnnouncingByzantiumAwareOfPetersburg() { // 6 Local is mainnet Petersburg, remote announces Byzantium + knowledge about Petersburg. // Remote is simply out of sync, accept. // {7987396, ID{Hash: 0x668db0af, Next: 7280000}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", 7280000L)); assertThat(result).isTrue(); } @@ -212,10 +199,9 @@ public void check7PetersburgWithRemoteAnnouncingSpuriousAwareOfByzantiumRemoteMa // Remote is definitely out of sync. It may or may not need the Petersburg update, we don't know // yet. // {7987396, ID{Hash: 0x3edd5b10, Next: 4370000}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x3edd5b10", 4370000L)); assertThat(result).isTrue(); } @@ -224,10 +210,9 @@ public void check7PetersburgWithRemoteAnnouncingSpuriousAwareOfByzantiumRemoteMa public void check8ByzantiumWithRemoteAnnouncingPetersburgLocalOutOfSync() { // 8 Local is mainnet Byzantium, remote announces Petersburg. Local is out of sync, accept. // {7279999, ID{Hash: 0x668db0af, Next: 0}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x668db0af", 0L)); assertThat(result).isTrue(); } @@ -237,10 +222,9 @@ public void check9SpuriousWithRemoteAnnouncingByzantiumRemoteUnawareOfPetersburg // 9 Local is mainnet Spurious, remote announces Byzantium, but is not aware of Petersburg. // Local out of sync. Local also knows about a future fork, but that is uncertain yet. // {4369999, ID{Hash: 0xa00bc324, Next: 0}, nil}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 4369999L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 4369999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 0L)); assertThat(result).isTrue(); } @@ -250,10 +234,9 @@ public void check10PetersburgWithRemoteAnnouncingByzantiumRemoteUnawareOfAdditio // 10 Local is mainnet Petersburg. remote announces Byzantium but is not aware of further forks. // Remote needs software update. // {7987396, ID{Hash: 0xa00bc324, Next: 0}, ErrRemoteStale}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xa00bc324", 0L)); assertThat(result).isFalse(); } @@ -263,10 +246,9 @@ public void check11PetersburgWithRemoteAnnouncingPetersburgAndFutureForkLocalNee // 11 Local is mainnet Petersburg, and isn't aware of more forks. Remote announces Petersburg + // 0xffffffff. Local needs software update, reject. // {7987396, ID{Hash: 0x5cddc0e1, Next: 0}, ErrLocalIncompatibleOrStale}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x5cddc0e1", 0L)); assertThat(result).isFalse(); } @@ -276,10 +258,9 @@ public void check12ByzantiumWithRemoteAnnouncingPetersburgAndFutureForkLocalNeed // 12 Local is mainnet Byzantium, and is aware of Petersburg. Remote announces Petersburg + // 0xffffffff. Local needs software update, reject. // {7279999, ID{Hash: 0x5cddc0e1, Next: 0}, ErrLocalIncompatibleOrStale}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7279999L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7279999L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0x5cddc0e1", 0L)); assertThat(result).isFalse(); } @@ -288,10 +269,9 @@ public void check12ByzantiumWithRemoteAnnouncingPetersburgAndFutureForkLocalNeed public void check13ByzantiumWithRemoteAnnouncingRinkebyPetersburg() { // 13 Local is mainnet Petersburg, remote is Rinkeby Petersburg. // {7987396, ID{Hash: 0xafec6b27, Next: 0}, ErrLocalIncompatibleOrStale}, - List list = Arrays.asList(forksMainnet); - Set forkSet = new LinkedHashSet<>(list); + List forkList = Arrays.asList(forksMainnet); ForkIdManager forkIdManager = - new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkSet, 7987396L); + new ForkIdManager(Hash.fromHexString(mainnetGenHash), forkList, 7987396L); Boolean result = forkIdManager.peerCheck(ForkIdManager.createIdEntry("0xafec6b27", 0L)); assertThat(result).isFalse(); }