From 6d7ebe1ba7c49e8969ea041de9b10b8d06269a33 Mon Sep 17 00:00:00 2001 From: mpicco Date: Thu, 27 Feb 2020 13:35:09 -0300 Subject: [PATCH 1/7] Introduce ummRoot field to BlockHeader and implement new hashForMergedMining computation method --- .../java/org/ethereum/core/BlockFactory.java | 3 +- .../java/org/ethereum/core/BlockHeader.java | 37 ++++- .../org/ethereum/core/BlockHeaderBuilder.java | 9 +- .../java/org/ethereum/core/GenesisHeader.java | 6 +- .../rsk/blockchain/utils/BlockGenerator.java | 1 - .../java/co/rsk/core/BlockFactoryTest.java | 24 +-- .../java/co/rsk/core/BlockHeaderTest.java | 154 ++++++++++++------ .../co/rsk/mine/BlockToMineBuilderTest.java | 2 +- .../java/co/rsk/remasc/RemascTestRunner.java | 2 +- .../ethereum/core/BlockHeaderBuilderTest.java | 17 ++ 10 files changed, 189 insertions(+), 66 deletions(-) diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java index a500427bacd..6c272590571 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java @@ -191,7 +191,8 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { number, glBytes, gasUsed, timestamp, extraData, paidFees, bitcoinMergedMiningHeader, bitcoinMergedMiningMerkleProof, bitcoinMergedMiningCoinbaseTransaction, new byte[0], - minimumGasPrice, uncleCount, sealed, useRskip92Encoding, includeForkDetectionData + minimumGasPrice, uncleCount, sealed, useRskip92Encoding, includeForkDetectionData, + null ); } diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java index 41466100d47..def9c606f28 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java @@ -47,6 +47,7 @@ public class BlockHeader { private static final int HASH_FOR_MERGED_MINING_PREFIX_LENGTH = 20; private static final int FORK_DETECTION_DATA_LENGTH = 12; + private static final int UMM_ROOT_LENGTH = 20; /* The SHA3 256-bit hash of the parent block, in its entirety */ private byte[] parentHash; @@ -102,6 +103,8 @@ public class BlockHeader { private byte[] miningForkDetectionData; + private byte[] ummRoot; + /** * The mgp for a tx to be included in the block. */ @@ -123,7 +126,7 @@ public BlockHeader(byte[] parentHash, byte[] unclesHash, RskAddress coinbase, by Coin paidFees, byte[] bitcoinMergedMiningHeader, byte[] bitcoinMergedMiningMerkleProof, byte[] bitcoinMergedMiningCoinbaseTransaction, byte[] mergedMiningForkDetectionData, Coin minimumGasPrice, int uncleCount, boolean sealed, - boolean useRskip92Encoding, boolean includeForkDetectionData) { + boolean useRskip92Encoding, boolean includeForkDetectionData, byte[] ummRoot) { this.parentHash = parentHash; this.unclesHash = unclesHash; this.coinbase = coinbase; @@ -148,6 +151,7 @@ public BlockHeader(byte[] parentHash, byte[] unclesHash, RskAddress coinbase, by this.sealed = sealed; this.useRskip92Encoding = useRskip92Encoding; this.includeForkDetectionData = includeForkDetectionData; + this.ummRoot = ummRoot; } @VisibleForTesting @@ -349,6 +353,10 @@ public byte[] getEncoded(boolean withMergedMiningFields, boolean withMerkleProof byte[] uncleCount = RLP.encodeBigInteger(BigInteger.valueOf(this.uncleCount)); fieldToEncodeList.add(uncleCount); + if (isUMMBlock()) { + fieldToEncodeList.add(RLP.encodeElement(this.ummRoot)); + } + if (withMergedMiningFields && hasMiningFields()) { byte[] bitcoinMergedMiningHeader = RLP.encodeElement(this.bitcoinMergedMiningHeader); fieldToEncodeList.add(bitcoinMergedMiningHeader); @@ -477,9 +485,18 @@ public String getShortHashForMergedMining() { return HashUtil.shortHash(getHashForMergedMining()); } + public boolean isUMMBlock() { + return this.ummRoot != null && this.ummRoot.length != 0; + } + public byte[] getHashForMergedMining() { byte[] encodedBlock = getEncoded(false, false); byte[] hashForMergedMining = HashUtil.keccak256(encodedBlock); + + if (isUMMBlock()) { + hashForMergedMining = this.getHashRootForMergedMining(hashForMergedMining); + } + if (includeForkDetectionData) { byte[] mergedMiningForkDetectionData = hasMiningFields() ? getMiningForkDetectionData() : @@ -496,6 +513,20 @@ public byte[] getHashForMergedMining() { return hashForMergedMining; } + private byte[] getHashRootForMergedMining(byte[] leftHash) { + if ((ummRoot.length != UMM_ROOT_LENGTH) && (ummRoot.length != 0)){ + throw new IllegalStateException( + String.format("UMM Root length must be either 0 or 20. Found: %d", ummRoot.length) + ); + } + + byte[] leftRight = Arrays.copyOf(leftHash, leftHash.length + ummRoot.length); + arraycopy(ummRoot, 0, leftRight, leftHash.length, ummRoot.length); + + byte[] root256 = HashUtil.keccak256(leftRight); + return root256; + } + public String getShortHash() { return HashUtil.shortHash(getHash().getBytes()); } @@ -541,4 +572,8 @@ public byte[] getMiningForkDetectionData() { public boolean isParentOf(BlockHeader header) { return this.getHash().equals(header.getParentHash()); } + + public byte[] getUmmRoot() { + return ummRoot; + } } diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java index ccf52a14eb9..0a13c1907b0 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java @@ -56,6 +56,7 @@ public class BlockHeaderBuilder { private byte[] bitcoinMergedMiningMerkleProof; private byte[] bitcoinMergedMiningCoinbaseTransaction; private byte[] mergedMiningForkDetectionData; + private byte[] ummRoot; private Coin minimumGasPrice; private int uncleCount; @@ -239,6 +240,11 @@ public BlockHeaderBuilder setEmptyReceiptTrieRoot() { return this; } + public BlockHeaderBuilder setUmmRoot(byte[] ummRoot) { + this.ummRoot = copy(ummRoot); + return this; + } + private void initializeWithDefaultValues() { extraData = normalizeValue(extraData, new byte[0]); bitcoinMergedMiningHeader = normalizeValue(bitcoinMergedMiningHeader, new byte[0]); @@ -255,6 +261,7 @@ private void initializeWithDefaultValues() { minimumGasPrice = normalizeValue(minimumGasPrice, Coin.ZERO); mergedMiningForkDetectionData = normalizeValue(mergedMiningForkDetectionData, new byte[12]); + ummRoot = normalizeValue(ummRoot, new byte[0]); } private T normalizeValue(T value, T defaultValue) { @@ -294,7 +301,7 @@ public BlockHeader build() { mergedMiningForkDetectionData, minimumGasPrice, uncleCount, false, useRskip92Encoding, - includeForkDetectionData + includeForkDetectionData, ummRoot ); } } \ No newline at end of file diff --git a/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java b/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java index d33b2d6a73f..1419ff2c978 100644 --- a/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java @@ -51,7 +51,8 @@ public GenesisHeader(byte[] parentHash, 0, false, useRskip92Encoding, - false); + false, + null); this.difficulty = ByteUtils.clone(difficulty); } @@ -93,7 +94,8 @@ public GenesisHeader(byte[] parentHash, 0, false, useRskip92Encoding, - false); + false, + null); this.difficulty = ByteUtils.clone(difficulty); } diff --git a/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java b/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java index 2e281adac27..2c642770f2b 100644 --- a/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java +++ b/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java @@ -169,7 +169,6 @@ public Block createChildBlock(Block parent, long fees, List uncles, Collections.emptyList(), uncles ); -// return createChildBlock(parent, 0); } public Block createChildBlock(Block parent, List txs, byte[] stateRoot) { diff --git a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java index c70d1e0c86e..ca87b1e305a 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java @@ -25,7 +25,6 @@ import org.ethereum.TestUtils; import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.config.blockchain.upgrades.ConsensusRule; -import org.ethereum.core.Block; import org.ethereum.core.BlockFactory; import org.ethereum.core.BlockHeader; import org.ethereum.core.Bloom; @@ -38,8 +37,7 @@ import java.math.BigInteger; import java.util.Arrays; -import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP110; -import static org.ethereum.config.blockchain.upgrades.ConsensusRule.RSKIP92; +import static org.ethereum.config.blockchain.upgrades.ConsensusRule.*; import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import static org.mockito.AdditionalMatchers.geq; @@ -69,7 +67,7 @@ public void newHeaderWithNoForkDetectionDataAndRskip110On() { long number = 20L; enableRulesAt(number, RSKIP92, RSKIP110); - BlockHeader header = createBlockHeader(number, new byte[0]); + BlockHeader header = createBlockHeader(number, new byte[0], new byte[0]); Keccak256 hash = header.getHash(); byte[] hashForMergedMining = header.getHashForMergedMining(); @@ -82,7 +80,7 @@ public void decodeBlockPriorToHeight449AndRskip110On() { long number = 20L; enableRulesAt(number, RSKIP92, RSKIP110); - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0]); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], new byte[0]); byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -98,7 +96,7 @@ public void decodeBlockPriorToHeight449AndRskip110Off() { long number = 20L; enableRulesAt(number, RSKIP92); - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0]); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], new byte[0]); byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -114,7 +112,7 @@ public void decodeBlockAfterHeight449AndRskip110OFF() { long number = 457L; enableRulesAt(number, RSKIP92); - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0]); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], new byte[0]); byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -131,7 +129,7 @@ public void decodeBlockAfterHeight449AndRskip110On() { enableRulesAt(number, RSKIP92, RSKIP110); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, forkDetectionData); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, forkDetectionData, new byte[0]); byte[] encodedBlock = header.getEncoded(false, false); byte[] hashForMergedMining = Arrays.copyOfRange(HashUtil.keccak256(encodedBlock), 0, 20); @@ -158,7 +156,7 @@ public void decodeWithNoMergedMiningDataAndRskip110OffAndNoForkDetectionData() { long number = 20L; enableRulesAt(number, RSKIP92); - BlockHeader header = createBlockHeader(number, new byte[0]); + BlockHeader header = createBlockHeader(number, new byte[0], new byte[0]); byte[] encodedHeader = header.getEncoded(false, false); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -179,7 +177,7 @@ public void decodeWithNoMergedMiningDataAndRskip110OffAndForkDetectionData() { enableRulesAt(number, RSKIP92); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeader(number, forkDetectionData); + BlockHeader header = createBlockHeader(number, forkDetectionData, new byte[0]); byte[] encodedHeader = header.getEncoded(false, false); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -197,7 +195,8 @@ private void enableRulesAt(long number, ConsensusRule... consensusRules) { private BlockHeader createBlockHeaderWithMergedMiningFields( long number, - byte[] forkDetectionData) { + byte[] forkDetectionData, + byte[] ummRoot) { byte[] difficulty = BigInteger.ONE.toByteArray(); byte[] gasLimit = BigInteger.valueOf(6800000).toByteArray(); long timestamp = 7731067; // Friday, 10 May 2019 6:04:05 @@ -227,7 +226,8 @@ private BlockHeader createBlockHeaderWithMergedMiningFields( private BlockHeader createBlockHeader( long number, - byte[] forkDetectionData) { + byte[] forkDetectionData, + byte[] ummRoot) { byte[] difficulty = BigInteger.ONE.toByteArray(); byte[] gasLimit = BigInteger.valueOf(6800000).toByteArray(); long timestamp = 7731067; // Friday, 10 May 2019 6:04:05 diff --git a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java index 4fc2659d1f1..df711f81a39 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java @@ -33,9 +33,9 @@ import java.math.BigInteger; import java.util.Arrays; +import static java.lang.System.arraycopy; import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; public class BlockHeaderTest { @Test @@ -45,7 +45,7 @@ public void getHashForMergedMiningWithForkDetectionDataAndIncludedOnAndMergedMin byte[] encodedBlock = header.getEncoded(false, false); byte[] hashForMergedMining = Arrays.copyOfRange(HashUtil.keccak256(encodedBlock), 0, 20); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - byte[] coinbase = org.bouncycastle.util.Arrays.concatenate(hashForMergedMining, forkDetectionData); + byte[] coinbase = concatenate(hashForMergedMining, forkDetectionData); header.setBitcoinMergedMiningCoinbaseTransaction(coinbase); header.seal(); @@ -67,7 +67,7 @@ public void getHashForMergedMiningWithNoForkDetectionDataAndIncludedOffAndMerged @Test public void getHashForMergedMiningWithForkDetectionDataAndIncludedOn() { byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeader(forkDetectionData, true); + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(forkDetectionData, true); byte[] hash = header.getHash().getBytes(); byte[] hashFirst20Elements = Arrays.copyOfRange(hash, 0, 20); @@ -81,7 +81,7 @@ public void getHashForMergedMiningWithForkDetectionDataAndIncludedOn() { @Test public void getHashForMergedMiningWithForkDetectionDataAndIncludedOff() { byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeader(forkDetectionData, false); + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(forkDetectionData, false); byte[] hash = header.getHash().getBytes(); byte[] hashForMergedMining = header.getHashForMergedMining(); @@ -101,7 +101,7 @@ public void getEncodedWithMergedMiningFields() { @Test public void getEncodedWithoutMergedMiningFields() { - BlockHeader header = createBlockHeader(new byte[0], false); + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(new byte[0], false); byte[] headerEncoded = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(headerEncoded); @@ -116,14 +116,79 @@ public void getMiningForkDetectionData() { byte[] encodedBlock = header.getEncoded(false, false); byte[] hashForMergedMining = Arrays.copyOfRange(HashUtil.keccak256(encodedBlock), 0, 20); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - byte[] coinbase = org.bouncycastle.util.Arrays.concatenate(hashForMergedMining, forkDetectionData); - coinbase = org.bouncycastle.util.Arrays.concatenate(RskMiningConstants.RSK_TAG, coinbase); + byte[] coinbase = concatenate(hashForMergedMining, forkDetectionData); + coinbase = concatenate(RskMiningConstants.RSK_TAG, coinbase); header.setBitcoinMergedMiningCoinbaseTransaction(coinbase); header.seal(); assertThat(forkDetectionData, is(header.getMiningForkDetectionData())); } + @Test + public void getUmmRoot() { + byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}; + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot); + + assertThat(header.getUmmRoot(), is(ummRoot)); + } + + @Test + public void isUMMBlockWhenUmmRoot() { + byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}; + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot); + + assertThat(header.isUMMBlock(), is(true)); + } + + @Test + public void isUMMBlockWhenNullUmmRoot() { + byte[] ummRoot = null; + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot); + + assertThat(header.isUMMBlock(), is(false)); + } + + @Test + public void isUMMBlockWhenEmptyUmmRoot() { + byte[] ummRoot = new byte[0]; + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot); + + assertThat(header.isUMMBlock(), is(false)); + } + + @Test + public void getHashForMergedMiningWhenUmmRoot() { + byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}; + byte[] forkDetectionData = new byte[]{20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31}; + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot, forkDetectionData); + + byte[] encodedBlock = header.getEncoded(false, false); + byte[] oldHashForMergedMining = HashUtil.keccak256(encodedBlock); + byte[] hashRoot = HashUtil.keccak256(concatenate(oldHashForMergedMining, ummRoot)); + byte[] newHashForMergedMining = concatenate( + java.util.Arrays.copyOfRange(hashRoot, 0, 20), + forkDetectionData + ); + + assertThat(header.getHashForMergedMining(), is(newHashForMergedMining)); + } + + @Test(expected = IllegalStateException.class) + public void getHashForMergedMiningWhenUmmRootWithLengthUnder20() { + byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18}; + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot); + + header.getHashForMergedMining(); + } + + @Test(expected = IllegalStateException.class) + public void getHashForMergedMiningWhenUmmRootWithLengthOver20() { + byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20}; + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot); + + header.getHashForMergedMining(); + } + /** * This case is an error and should never happen in production */ @@ -132,7 +197,7 @@ public void getMiningForkDetectionDataNoDataCanBeFound() { BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], true); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - byte[] coinbase = org.bouncycastle.util.Arrays.concatenate(RskMiningConstants.RSK_TAG, forkDetectionData); + byte[] coinbase = concatenate(RskMiningConstants.RSK_TAG, forkDetectionData); header.setBitcoinMergedMiningCoinbaseTransaction(coinbase); header.seal(); @@ -149,6 +214,30 @@ public void getMiningForkDetectionDataNoDataMustBeIncluded() { private BlockHeader createBlockHeaderWithMergedMiningFields( byte[] forkDetectionData, boolean includeForkDetectionData){ + return createBlockHeader(new byte[80], new byte[32], new byte[128], + forkDetectionData, includeForkDetectionData, null, false); + } + + private BlockHeader createBlockHeaderWithNoMergedMiningFields( + byte[] forkDetectionData, + boolean includeForkDetectionData) { + return createBlockHeader(null, null, null, + forkDetectionData, includeForkDetectionData, null, true); + } + + private BlockHeader createBlockHeaderWithUmmRoot(byte[] ummRoot) { + return createBlockHeader(null, null, null, + new byte[0], false, ummRoot, true); + } + + private BlockHeader createBlockHeaderWithUmmRoot(byte[] ummRoot, byte[] forkDetectionData) { + return createBlockHeader(null, null, null, + forkDetectionData, true, ummRoot, true); + } + + private BlockHeader createBlockHeader(byte[] bitcoinMergedMiningHeader, byte[] bitcoinMergedMiningMerkleProof, + byte[] bitcoinMergedMiningCoinbaseTransaction, byte[] forkDetectionData, + boolean includeForkDetectionData, byte[] ummRoot, boolean sealed) { BlockDifficulty difficulty = new BlockDifficulty(BigInteger.ONE); long number = 1; BigInteger gasLimit = BigInteger.valueOf(6800000); @@ -169,48 +258,21 @@ private BlockHeader createBlockHeaderWithMergedMiningFields( timestamp, new byte[0], Coin.ZERO, - new byte[80], - new byte[32], - new byte[128], + bitcoinMergedMiningHeader, + bitcoinMergedMiningMerkleProof, + bitcoinMergedMiningCoinbaseTransaction, forkDetectionData, Coin.valueOf(10L), 0, - false, + sealed, true, - includeForkDetectionData); + includeForkDetectionData, + ummRoot); } - private BlockHeader createBlockHeader( - byte[] forkDetectionData, - boolean includeForkDetectionData){ - BlockDifficulty difficulty = new BlockDifficulty(BigInteger.ONE); - long number = 1; - BigInteger gasLimit = BigInteger.valueOf(6800000); - long timestamp = 7731067; // Friday, 10 May 2019 6:04:05 - - return new BlockHeader( - PegTestUtils.createHash3().getBytes(), - HashUtil.keccak256(RLP.encodeList()), - new RskAddress(TestUtils.randomAddress().getBytes()), - HashUtil.EMPTY_TRIE_HASH, - "tx_trie_root".getBytes(), - HashUtil.EMPTY_TRIE_HASH, - new Bloom().getData(), - difficulty, - number, - gasLimit.toByteArray(), - 3000000, - timestamp, - new byte[0], - Coin.ZERO, - null, - null, - null, - forkDetectionData, - Coin.valueOf(10L), - 0, - true, - true, - includeForkDetectionData); + private byte[] concatenate(byte[] left, byte[] right) { + byte[] leftRight = Arrays.copyOf(left, left.length + right.length); + arraycopy(right, 0, leftRight, left.length, right.length); + return leftRight; } } diff --git a/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java b/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java index 0983ccd3cb7..58c89a2ceda 100644 --- a/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java +++ b/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java @@ -161,7 +161,7 @@ private BlockHeader createBlockHeader() { new Bloom().getData(), BlockDifficulty.ZERO, 1L, EMPTY_BYTE_ARRAY, 0L, 0L, EMPTY_BYTE_ARRAY, Coin.ZERO, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY, - Coin.ZERO, 0, false, true, false + Coin.ZERO, 0, false, true, false, null ); } } \ No newline at end of file diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java b/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java index 9f81fd50dd5..37458ee2362 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java @@ -294,7 +294,7 @@ public HardcodedHashBlockHeader( HashUtil.EMPTY_TRIE_HASH, new Bloom().getData(), finalDifficulty, parentBlock.getNumber() + 1, parentBlock.getGasLimit(), parentBlock.getGasUsed(), parentBlock.getTimestamp(), new byte[0], paidFees, null, null, null, new byte[0], - Coin.valueOf(10), uncles.size(), false, true, false + Coin.valueOf(10), uncles.size(), false, true, false, null ); this.blockHash = blockHash; } diff --git a/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java b/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java index a183a23fbe8..0123d61c8ed 100644 --- a/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java +++ b/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java @@ -359,4 +359,21 @@ public void createsHeaderWithEmptyMergedMiningForkDetectionData() { assertArrayEquals(new byte[12], header.getMiningForkDetectionData()); } + + @Test + public void createsHeaderWithUmmRoot() { + byte[] ummRoot = TestUtils.randomBytes(20); + BlockHeader header = blockHeaderBuilder + .setUmmRoot(ummRoot) + .build(); + + assertArrayEquals(ummRoot, header.getUmmRoot()); + } + + @Test + public void createsHeaderWithEmptyUmmRoot() { + BlockHeader header = blockHeaderBuilder.build(); + + assertArrayEquals(new byte[0], header.getUmmRoot()); + } } From 47149a817c8371b588a4656b1dc76e8387695cdd Mon Sep 17 00:00:00 2001 From: mpicco Date: Fri, 28 Feb 2020 15:11:04 -0300 Subject: [PATCH 2/7] Include ummRoot field to BlockHeader RLP encoding --- .../blockchain/upgrades/ConsensusRule.java | 3 +- .../java/org/ethereum/core/BlockFactory.java | 26 +++-- rskj-core/src/main/resources/reference.conf | 1 + .../java/co/rsk/core/BlockFactoryTest.java | 104 ++++++++++++++++++ .../upgrades/ActivationConfigTest.java | 1 + 5 files changed, 126 insertions(+), 9 deletions(-) diff --git a/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java b/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java index aba8ba0b847..c3e4bb29316 100644 --- a/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java +++ b/rskj-core/src/main/java/org/ethereum/config/blockchain/upgrades/ConsensusRule.java @@ -49,7 +49,8 @@ public enum ConsensusRule { RSKIP150("rskip150"), RSKIP151("rskip151"), RSKIP152("rskip152"), - RSKIP156("rskip156"); + RSKIP156("rskip156"), + RSKIPUMM("rskipUMM"); private String configKey; diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java index 6c272590571..f80e1618991 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java @@ -41,6 +41,9 @@ public class BlockFactory { private static final int RLP_HEADER_SIZE = 16; private static final int RLP_HEADER_SIZE_WITH_MERGED_MINING = 19; + private static final int RLP_HEADER_SIZE_UMM = 17; + private static final int RLP_HEADER_SIZE_UMM_WITH_MERGED_MINING = 20; + private final ActivationConfig activationConfig; public BlockFactory(ActivationConfig activationConfig) { @@ -97,10 +100,9 @@ public BlockHeader decodeHeader(byte[] encoded) { } private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { - // TODO fix old tests that have other sizes - if (rlpHeader.size() != RLP_HEADER_SIZE && rlpHeader.size() != RLP_HEADER_SIZE_WITH_MERGED_MINING) { + if (!canBeDecoded(rlpHeader)) { throw new IllegalArgumentException(String.format( - "A block header must have 16 elements or 19 including merged-mining fields but it had %d", + "A block header must have 16/17 elements or 19/20 including merged-mining fields but it had %d", rlpHeader.size() )); } @@ -146,10 +148,13 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { int r = 15; - int uncleCount = 0; - if (rlpHeader.size() == RLP_HEADER_SIZE || rlpHeader.size() == RLP_HEADER_SIZE_WITH_MERGED_MINING) { - byte[] ucBytes = rlpHeader.get(r++).getRLPData(); - uncleCount = parseBigInteger(ucBytes).intValueExact(); + byte[] ucBytes = rlpHeader.get(r++).getRLPData(); + int uncleCount = parseBigInteger(ucBytes).intValueExact(); + + byte[] ummRoot = new byte[0]; + if (activationConfig.isActive(ConsensusRule.RSKIPUMM, number) && (rlpHeader.size() == RLP_HEADER_SIZE_UMM || + rlpHeader.size() == RLP_HEADER_SIZE_UMM_WITH_MERGED_MINING)) { + ummRoot = rlpHeader.get(r++).getRLPRawData(); } byte[] bitcoinMergedMiningHeader = null; @@ -192,10 +197,15 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { paidFees, bitcoinMergedMiningHeader, bitcoinMergedMiningMerkleProof, bitcoinMergedMiningCoinbaseTransaction, new byte[0], minimumGasPrice, uncleCount, sealed, useRskip92Encoding, includeForkDetectionData, - null + ummRoot ); } + private boolean canBeDecoded(RLPList rlpHeader) { + return rlpHeader.size() == RLP_HEADER_SIZE || rlpHeader.size() == RLP_HEADER_SIZE_WITH_MERGED_MINING || + rlpHeader.size() == RLP_HEADER_SIZE_UMM || rlpHeader.size() == RLP_HEADER_SIZE_UMM_WITH_MERGED_MINING; + } + private static BigInteger parseBigInteger(byte[] bytes) { return bytes == null ? BigInteger.ZERO : BigIntegers.fromUnsignedByteArray(bytes); } diff --git a/rskj-core/src/main/resources/reference.conf b/rskj-core/src/main/resources/reference.conf index e4890bf9be8..63ed2221528 100644 --- a/rskj-core/src/main/resources/reference.conf +++ b/rskj-core/src/main/resources/reference.conf @@ -35,6 +35,7 @@ blockchain = { rskip151 = papyrus200, rskip152 = papyrus200 rskip156 = papyrus200 + rskipUMM = papyrus200 } } gc = { diff --git a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java index ca87b1e305a..0b15ba18514 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java @@ -187,6 +187,110 @@ public void decodeWithNoMergedMiningDataAndRskip110OffAndForkDetectionData() { assertThat(header.getHash(), is(decodedHeader.getHash())); } + @Test + public void decodeBlockRskip110OffRskipUMMOnAndNoMergedMiningFieldsValidUMMRoot() { + long number = 500L; + enableRulesAt(number, RSKIP92, RSKIPUMM); + + byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}; + BlockHeader header = createBlockHeader(number, new byte[0], ummRoot); + + byte[] encodedHeader = header.getEncoded(); + RLPList headerRLP = RLP.decodeList(encodedHeader); + assertThat(headerRLP.size(), is(17)); + + BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); + + assertThat(header.getHash(), is(decodedHeader.getHash())); + assertThat(header.getUmmRoot(), is(decodedHeader.getUmmRoot())); + } + + @Test + public void decodeBlockRskip110OffRskipUMMOnAndNoMergedMiningFieldsEmptyUMMRoot() { + long number = 500L; + enableRulesAt(number, RSKIP92, RSKIPUMM); + + BlockHeader header = createBlockHeader(number, new byte[0], new byte[0]); + + byte[] encodedHeader = header.getEncoded(); + RLPList headerRLP = RLP.decodeList(encodedHeader); + assertThat(headerRLP.size(), is(16)); + + BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); + + assertThat(header.getHash(), is(decodedHeader.getHash())); + assertThat(header.getUmmRoot(), is(decodedHeader.getUmmRoot())); + } + + @Test + public void decodeBlockRskip110OffRskipUMMOnAndNoMergedMiningFieldsNullUMMRoot() { + long number = 500L; + enableRulesAt(number, RSKIP92, RSKIPUMM); + + BlockHeader header = createBlockHeader(number, new byte[0], null); + + byte[] encodedHeader = header.getEncoded(); + RLPList headerRLP = RLP.decodeList(encodedHeader); + assertThat(headerRLP.size(), is(16)); + + BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); + + assertThat(decodedHeader.getHash(), is(header.getHash())); + assertThat(decodedHeader.getUmmRoot(), is(new byte[0])); + } + + @Test + public void decodeBlockRskip110OffRskipUMMOnAndMergedMiningFieldsValidUMMRoot() { + long number = 500L; + enableRulesAt(number, RSKIP92, RSKIPUMM); + + byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}; + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], ummRoot); + + byte[] encodedHeader = header.getFullEncoded(); + RLPList headerRLP = RLP.decodeList(encodedHeader); + assertThat(headerRLP.size(), is(20)); + + BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); + + assertThat(header.getHash(), is(decodedHeader.getHash())); + assertThat(header.getUmmRoot(), is(decodedHeader.getUmmRoot())); + } + + @Test + public void decodeBlockRskip110OffRskipUMMOnAndMergedMiningFieldsEmptyUmmRoot() { + long number = 500L; + enableRulesAt(number, RSKIP92, RSKIPUMM); + + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], new byte[0]); + + byte[] encodedHeader = header.getFullEncoded(); + RLPList headerRLP = RLP.decodeList(encodedHeader); + assertThat(headerRLP.size(), is(19)); + + BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); + + assertThat(header.getHash(), is(decodedHeader.getHash())); + assertThat(header.getUmmRoot(), is(decodedHeader.getUmmRoot())); + } + + @Test + public void decodeBlockRskip110OffRskipUMMOnAndMergedMiningFieldsNullUmmRoot() { + long number = 500L; + enableRulesAt(number, RSKIP92, RSKIPUMM); + + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], null); + + byte[] encodedHeader = header.getFullEncoded(); + RLPList headerRLP = RLP.decodeList(encodedHeader); + assertThat(headerRLP.size(), is(19)); + + BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); + + assertThat(decodedHeader.getHash(), is(header.getHash())); + assertThat(decodedHeader.getUmmRoot(), is(new byte[0])); + } + private void enableRulesAt(long number, ConsensusRule... consensusRules) { for (ConsensusRule consensusRule : consensusRules) { when(activationConfig.isActive(eq(consensusRule), geq(number))).thenReturn(true); diff --git a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java index 44984893e1b..43a2bd863ef 100644 --- a/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java +++ b/rskj-core/src/test/java/org/ethereum/config/blockchain/upgrades/ActivationConfigTest.java @@ -70,6 +70,7 @@ public class ActivationConfigTest { " rskip151: papyrus200", " rskip152: papyrus200", " rskip156: papyrus200", + " rskipUMM: papyrus200", "}" )); From b24cbd468cabc48a13ca1abc8a3349435552575c Mon Sep 17 00:00:00 2001 From: mpicco Date: Mon, 2 Mar 2020 11:06:42 -0300 Subject: [PATCH 3/7] ummRoot field creation in BlockToMineBuilder --- .../java/co/rsk/mine/BlockToMineBuilder.java | 4 ++ .../java/org/ethereum/core/BlockHeader.java | 4 +- .../java/org/ethereum/core/GenesisHeader.java | 4 +- .../java/co/rsk/core/BlockFactoryTest.java | 2 + .../co/rsk/mine/BlockToMineBuilderTest.java | 55 ++++++++++++++++++- .../java/co/rsk/remasc/RemascTestRunner.java | 2 +- 6 files changed, 64 insertions(+), 7 deletions(-) diff --git a/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java b/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java index 05d7ee3c5e0..c8ed5857665 100644 --- a/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java +++ b/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java @@ -196,6 +196,9 @@ private BlockHeader createHeader( long blockNumber = newBlockParentHeader.getNumber() + 1; + // ummRoot can not be set to a value yet since the UMM contracts are not yet implemented + byte[] ummRoot = new byte[0]; + final BlockHeader newHeader = blockFactory .getBlockHeaderBuilder() .setParentHash(newBlockParentHeader.getHash().getBytes()) @@ -214,6 +217,7 @@ private BlockHeader createHeader( .setMergedMiningForkDetectionData(forkDetectionData) .setMinimumGasPrice(minimumGasPrice) .setUncleCount(uncles.size()) + .setUmmRoot(ummRoot) .build(); newHeader.setDifficulty(difficultyCalculator.calcDifficulty(newHeader, newBlockParentHeader)); diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java index def9c606f28..8732a293617 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java @@ -151,7 +151,7 @@ public BlockHeader(byte[] parentHash, byte[] unclesHash, RskAddress coinbase, by this.sealed = sealed; this.useRskip92Encoding = useRskip92Encoding; this.includeForkDetectionData = includeForkDetectionData; - this.ummRoot = ummRoot; + this.ummRoot = ummRoot != null ? Arrays.copyOf(ummRoot, ummRoot.length) : new byte[0]; } @VisibleForTesting @@ -574,6 +574,6 @@ public boolean isParentOf(BlockHeader header) { } public byte[] getUmmRoot() { - return ummRoot; + return Arrays.copyOf(ummRoot, ummRoot.length); } } diff --git a/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java b/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java index 1419ff2c978..07bfb0f55a1 100644 --- a/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java @@ -52,7 +52,7 @@ public GenesisHeader(byte[] parentHash, false, useRskip92Encoding, false, - null); + new byte[0]); this.difficulty = ByteUtils.clone(difficulty); } @@ -95,7 +95,7 @@ public GenesisHeader(byte[] parentHash, false, useRskip92Encoding, false, - null); + new byte[0]); this.difficulty = ByteUtils.clone(difficulty); } diff --git a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java index 0b15ba18514..93327330d0a 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java @@ -325,6 +325,7 @@ private BlockHeader createBlockHeaderWithMergedMiningFields( .setMergedMiningForkDetectionData(forkDetectionData) .setMinimumGasPrice(Coin.valueOf(10L)) .setUncleCount(0) + .setUmmRoot(ummRoot) .build(); } @@ -353,6 +354,7 @@ private BlockHeader createBlockHeader( .setMergedMiningForkDetectionData(forkDetectionData) .setMinimumGasPrice(Coin.valueOf(10L)) .setUncleCount(0) + .setUmmRoot(ummRoot) .build(); } diff --git a/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java b/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java index 58c89a2ceda..a66a9adae97 100644 --- a/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java +++ b/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java @@ -34,6 +34,7 @@ import org.ethereum.config.Constants; import org.ethereum.config.blockchain.upgrades.ActivationConfig; import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; +import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.core.*; import org.ethereum.db.BlockStore; import org.junit.Before; @@ -50,6 +51,7 @@ import static org.ethereum.crypto.HashUtil.EMPTY_TRIE_HASH; import static org.ethereum.util.ByteUtil.EMPTY_BYTE_ARRAY; import static org.hamcrest.Matchers.*; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.mockito.Mockito.*; @@ -60,6 +62,7 @@ public class BlockToMineBuilderTest { private BlockToMineBuilder blockBuilder; private BlockValidationRule validationRules; private BlockExecutor blockExecutor; + private ActivationConfig activationConfig; @Before public void setUp() { @@ -71,10 +74,11 @@ public void setUp() { DifficultyCalculator difficultyCalculator = mock(DifficultyCalculator.class); MinimumGasPriceCalculator minimumGasPriceCalculator = mock(MinimumGasPriceCalculator.class); MinerUtils minerUtils = mock(MinerUtils.class); + activationConfig = mock(ActivationConfig.class); blockExecutor = mock(BlockExecutor.class); blockBuilder = new BlockToMineBuilder( - mock(ActivationConfig.class), + activationConfig, miningConfig, repositoryLocator, mock(BlockStore.class), @@ -134,6 +138,52 @@ public void BuildBlockHasUnclesWhenCreateAnInvalidBlock() { assertThat(blockCaptor.getValue().getUncleList(), hasSize(1)); } + @Test + public void buildBlockBeforeUMMActivation() { + Keccak256 parentHash = TestUtils.randomHash(); + + BlockHeader parent = mock(BlockHeader.class); + when(parent.getNumber()).thenReturn(500L); + when(parent.getHash()).thenReturn(parentHash); + when(parent.getGasLimit()).thenReturn(new byte[0]); + when(parent.getMinimumGasPrice()).thenReturn(mock(Coin.class)); + + when(validationRules.isValid(any())).thenReturn(true); + when(activationConfig.isActive(ConsensusRule.RSKIPUMM, 501L)).thenReturn(false); + + BlockResult expectedResult = mock(BlockResult.class); + ArgumentCaptor blockCaptor = ArgumentCaptor.forClass(Block.class); + when(blockExecutor.executeAndFill(blockCaptor.capture(), any())).thenReturn(expectedResult); + + blockBuilder.build(new ArrayList<>(Collections.singletonList(parent)), new byte[0]); + + Block actualBlock = blockCaptor.getValue(); + assertThat(actualBlock.getHeader().getUmmRoot(), is(new byte[0])); + } + + @Test + public void buildBlockAfterUMMActivation() { + Keccak256 parentHash = TestUtils.randomHash(); + + BlockHeader parent = mock(BlockHeader.class); + when(parent.getNumber()).thenReturn(500L); + when(parent.getHash()).thenReturn(parentHash); + when(parent.getGasLimit()).thenReturn(new byte[0]); + when(parent.getMinimumGasPrice()).thenReturn(mock(Coin.class)); + + when(validationRules.isValid(any())).thenReturn(true); + when(activationConfig.isActive(ConsensusRule.RSKIPUMM, 501L)).thenReturn(true); + + BlockResult expectedResult = mock(BlockResult.class); + ArgumentCaptor blockCaptor = ArgumentCaptor.forClass(Block.class); + when(blockExecutor.executeAndFill(blockCaptor.capture(), any())).thenReturn(expectedResult); + + blockBuilder.build(new ArrayList<>(Collections.singletonList(parent)), new byte[0]); + + Block actualBlock = blockCaptor.getValue(); + assertThat(actualBlock.getHeader().getUmmRoot(), is(new byte[0])); + } + private BlockHeader buildBlockHeaderWithSibling() { BlockHeader blockHeader = mock(BlockHeader.class); long blockNumber = 42L; @@ -161,7 +211,8 @@ private BlockHeader createBlockHeader() { new Bloom().getData(), BlockDifficulty.ZERO, 1L, EMPTY_BYTE_ARRAY, 0L, 0L, EMPTY_BYTE_ARRAY, Coin.ZERO, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY, EMPTY_BYTE_ARRAY, - Coin.ZERO, 0, false, true, false, null + Coin.ZERO, 0, false, true, false, + new byte[0] ); } } \ No newline at end of file diff --git a/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java b/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java index 37458ee2362..ee17b5e7c77 100644 --- a/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java +++ b/rskj-core/src/test/java/co/rsk/remasc/RemascTestRunner.java @@ -294,7 +294,7 @@ public HardcodedHashBlockHeader( HashUtil.EMPTY_TRIE_HASH, new Bloom().getData(), finalDifficulty, parentBlock.getNumber() + 1, parentBlock.getGasLimit(), parentBlock.getGasUsed(), parentBlock.getTimestamp(), new byte[0], paidFees, null, null, null, new byte[0], - Coin.valueOf(10), uncles.size(), false, true, false, null + Coin.valueOf(10), uncles.size(), false, true, false, new byte[0] ); this.blockHash = blockHash; } From a042173633b0a66ec06bac402375a7650204b403 Mon Sep 17 00:00:00 2001 From: mpicco Date: Mon, 9 Mar 2020 16:57:49 -0300 Subject: [PATCH 4/7] Allow ummRoot to take null values in order to differentiate blocks between pre and post umm activation. Those before umm activation will have null ummRoot, while those after take an empty byte array as value --- .../java/co/rsk/mine/BlockToMineBuilder.java | 2 +- .../java/org/ethereum/core/BlockFactory.java | 2 +- .../java/org/ethereum/core/BlockHeader.java | 6 +- .../org/ethereum/core/BlockHeaderBuilder.java | 16 +++- .../java/org/ethereum/core/GenesisHeader.java | 4 +- .../rsk/blockchain/utils/BlockGenerator.java | 1 + .../java/co/rsk/core/BlockFactoryTest.java | 26 +++---- .../co/rsk/core/BlockFactoryTest.java.rej | 27 +++++++ .../java/co/rsk/core/BlockHeaderTest.java | 73 +++++++++++++++---- .../src/test/java/co/rsk/core/BlockTest.java | 1 + .../co/rsk/mine/BlockToMineBuilderTest.java | 4 +- .../ethereum/core/BlockHeaderBuilderTest.java | 17 ++++- 12 files changed, 135 insertions(+), 44 deletions(-) create mode 100644 rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java.rej diff --git a/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java b/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java index c8ed5857665..a66c273c9e8 100644 --- a/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java +++ b/rskj-core/src/main/java/co/rsk/mine/BlockToMineBuilder.java @@ -197,7 +197,7 @@ private BlockHeader createHeader( long blockNumber = newBlockParentHeader.getNumber() + 1; // ummRoot can not be set to a value yet since the UMM contracts are not yet implemented - byte[] ummRoot = new byte[0]; + byte[] ummRoot = activationConfig.isActive(ConsensusRule.RSKIPUMM, blockNumber) ? new byte[0] : null; final BlockHeader newHeader = blockFactory .getBlockHeaderBuilder() diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java index f80e1618991..99978b994df 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java @@ -151,7 +151,7 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { byte[] ucBytes = rlpHeader.get(r++).getRLPData(); int uncleCount = parseBigInteger(ucBytes).intValueExact(); - byte[] ummRoot = new byte[0]; + byte[] ummRoot = null; if (activationConfig.isActive(ConsensusRule.RSKIPUMM, number) && (rlpHeader.size() == RLP_HEADER_SIZE_UMM || rlpHeader.size() == RLP_HEADER_SIZE_UMM_WITH_MERGED_MINING)) { ummRoot = rlpHeader.get(r++).getRLPRawData(); diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java index 8732a293617..217e1f86a95 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java @@ -151,7 +151,7 @@ public BlockHeader(byte[] parentHash, byte[] unclesHash, RskAddress coinbase, by this.sealed = sealed; this.useRskip92Encoding = useRskip92Encoding; this.includeForkDetectionData = includeForkDetectionData; - this.ummRoot = ummRoot != null ? Arrays.copyOf(ummRoot, ummRoot.length) : new byte[0]; + this.ummRoot = ummRoot != null ? Arrays.copyOf(ummRoot, ummRoot.length) : null; } @VisibleForTesting @@ -353,7 +353,7 @@ public byte[] getEncoded(boolean withMergedMiningFields, boolean withMerkleProof byte[] uncleCount = RLP.encodeBigInteger(BigInteger.valueOf(this.uncleCount)); fieldToEncodeList.add(uncleCount); - if (isUMMBlock()) { + if (this.ummRoot != null) { fieldToEncodeList.add(RLP.encodeElement(this.ummRoot)); } @@ -574,6 +574,6 @@ public boolean isParentOf(BlockHeader header) { } public byte[] getUmmRoot() { - return Arrays.copyOf(ummRoot, ummRoot.length); + return ummRoot != null ? Arrays.copyOf(ummRoot, ummRoot.length) : null; } } diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java index 0a13c1907b0..da2c03428fc 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java @@ -241,7 +241,7 @@ public BlockHeaderBuilder setEmptyReceiptTrieRoot() { } public BlockHeaderBuilder setUmmRoot(byte[] ummRoot) { - this.ummRoot = copy(ummRoot); + this.ummRoot = copy(ummRoot, null); return this; } @@ -261,7 +261,6 @@ private void initializeWithDefaultValues() { minimumGasPrice = normalizeValue(minimumGasPrice, Coin.ZERO); mergedMiningForkDetectionData = normalizeValue(mergedMiningForkDetectionData, new byte[12]); - ummRoot = normalizeValue(ummRoot, new byte[0]); } private T normalizeValue(T value, T defaultValue) { @@ -273,8 +272,12 @@ private byte[] copy(Keccak256 hash) { } private byte[] copy(byte[] bytes) { + return copy(bytes, new byte[0]); + } + + private byte[] copy(byte[] bytes, byte[] defaultValue) { if (bytes == null) { - return new byte[0]; + return defaultValue; } return Arrays.copyOf(bytes, bytes.length); @@ -284,12 +287,19 @@ public BlockHeader build() { // Initial null values in some fields are replaced by empty // arrays initializeWithDefaultValues(); + if (createConsensusCompliantHeader) { useRskip92Encoding = activationConfig.isActive(ConsensusRule.RSKIP92, number); includeForkDetectionData = activationConfig.isActive(ConsensusRule.RSKIP110, number) && mergedMiningForkDetectionData.length > 0; } + if (activationConfig.isActive(ConsensusRule.RSKIPUMM, number)) { + if (ummRoot == null) { + ummRoot = new byte[0]; + } + } + return new BlockHeader( parentHash, unclesHash, coinbase, stateRoot, txTrieRoot, receiptTrieRoot, diff --git a/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java b/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java index 07bfb0f55a1..1419ff2c978 100644 --- a/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/GenesisHeader.java @@ -52,7 +52,7 @@ public GenesisHeader(byte[] parentHash, false, useRskip92Encoding, false, - new byte[0]); + null); this.difficulty = ByteUtils.clone(difficulty); } @@ -95,7 +95,7 @@ public GenesisHeader(byte[] parentHash, false, useRskip92Encoding, false, - new byte[0]); + null); this.difficulty = ByteUtils.clone(difficulty); } diff --git a/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java b/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java index 2c642770f2b..485571fa46e 100644 --- a/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java +++ b/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java @@ -162,6 +162,7 @@ public Block createChildBlock(Block parent, long fees, List uncles, .setPaidFees(Coin.valueOf(fees)) .setEmptyMergedMiningForkDetectionData() .setUncleCount(uncles.size()) + .setUmmRoot(new byte[0]) .build(); return blockFactory.newBlock( diff --git a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java index 93327330d0a..79fd5fc252a 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java @@ -39,7 +39,7 @@ import static org.ethereum.config.blockchain.upgrades.ConsensusRule.*; import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; import static org.mockito.AdditionalMatchers.geq; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -80,7 +80,7 @@ public void decodeBlockPriorToHeight449AndRskip110On() { long number = 20L; enableRulesAt(number, RSKIP92, RSKIP110); - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], new byte[0]); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], null); byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -96,7 +96,7 @@ public void decodeBlockPriorToHeight449AndRskip110Off() { long number = 20L; enableRulesAt(number, RSKIP92); - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], new byte[0]); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], null); byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -112,7 +112,7 @@ public void decodeBlockAfterHeight449AndRskip110OFF() { long number = 457L; enableRulesAt(number, RSKIP92); - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], new byte[0]); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, new byte[0], null); byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -129,7 +129,7 @@ public void decodeBlockAfterHeight449AndRskip110On() { enableRulesAt(number, RSKIP92, RSKIP110); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeaderWithMergedMiningFields(number, forkDetectionData, new byte[0]); + BlockHeader header = createBlockHeaderWithMergedMiningFields(number, forkDetectionData, null); byte[] encodedBlock = header.getEncoded(false, false); byte[] hashForMergedMining = Arrays.copyOfRange(HashUtil.keccak256(encodedBlock), 0, 20); @@ -156,7 +156,7 @@ public void decodeWithNoMergedMiningDataAndRskip110OffAndNoForkDetectionData() { long number = 20L; enableRulesAt(number, RSKIP92); - BlockHeader header = createBlockHeader(number, new byte[0], new byte[0]); + BlockHeader header = createBlockHeader(number, new byte[0], null); byte[] encodedHeader = header.getEncoded(false, false); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -177,7 +177,7 @@ public void decodeWithNoMergedMiningDataAndRskip110OffAndForkDetectionData() { enableRulesAt(number, RSKIP92); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeader(number, forkDetectionData, new byte[0]); + BlockHeader header = createBlockHeader(number, forkDetectionData, null); byte[] encodedHeader = header.getEncoded(false, false); RLPList headerRLP = RLP.decodeList(encodedHeader); @@ -214,7 +214,7 @@ public void decodeBlockRskip110OffRskipUMMOnAndNoMergedMiningFieldsEmptyUMMRoot( byte[] encodedHeader = header.getEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); - assertThat(headerRLP.size(), is(16)); + assertThat(headerRLP.size(), is(17)); BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); @@ -231,12 +231,12 @@ public void decodeBlockRskip110OffRskipUMMOnAndNoMergedMiningFieldsNullUMMRoot() byte[] encodedHeader = header.getEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); - assertThat(headerRLP.size(), is(16)); + assertThat(headerRLP.size(), is(17)); BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); assertThat(decodedHeader.getHash(), is(header.getHash())); - assertThat(decodedHeader.getUmmRoot(), is(new byte[0])); + assertArrayEquals(new byte[0], decodedHeader.getUmmRoot()); } @Test @@ -266,7 +266,7 @@ public void decodeBlockRskip110OffRskipUMMOnAndMergedMiningFieldsEmptyUmmRoot() byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); - assertThat(headerRLP.size(), is(19)); + assertThat(headerRLP.size(), is(20)); BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); @@ -283,12 +283,12 @@ public void decodeBlockRskip110OffRskipUMMOnAndMergedMiningFieldsNullUmmRoot() { byte[] encodedHeader = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(encodedHeader); - assertThat(headerRLP.size(), is(19)); + assertThat(headerRLP.size(), is(20)); BlockHeader decodedHeader = factory.decodeHeader(encodedHeader); assertThat(decodedHeader.getHash(), is(header.getHash())); - assertThat(decodedHeader.getUmmRoot(), is(new byte[0])); + assertArrayEquals(new byte[0], decodedHeader.getUmmRoot()); } private void enableRulesAt(long number, ConsensusRule... consensusRules) { diff --git a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java.rej b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java.rej new file mode 100644 index 00000000000..65bff49ccad --- /dev/null +++ b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java.rej @@ -0,0 +1,27 @@ +diff a/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java b/rskj-core/src/test/java/co/rsk/core/BlockFactoryTest.java (rejected hunks) +@@ -299,7 +299,8 @@ public class BlockFactoryTest { + + private BlockHeader createBlockHeaderWithMergedMiningFields( + long number, +- byte[] forkDetectionData) { ++ byte[] forkDetectionData, ++ byte[] ummRoot) { + byte[] difficulty = BigInteger.ONE.toByteArray(); + byte[] gasLimit = BigInteger.valueOf(6800000).toByteArray(); + long timestamp = 7731067; // Friday, 10 May 2019 6:04:05 +@@ -324,12 +325,14 @@ public class BlockFactoryTest { + .setMergedMiningForkDetectionData(forkDetectionData) + .setMinimumGasPrice(Coin.valueOf(10L)) + .setUncleCount(0) ++ .setUmmRoot(ummRoot) + .build(); + } + + private BlockHeader createBlockHeader( + long number, +- byte[] forkDetectionData) { ++ byte[] forkDetectionData, ++ byte[] ummRoot) { + byte[] difficulty = BigInteger.ONE.toByteArray(); + byte[] gasLimit = BigInteger.valueOf(6800000).toByteArray(); + long timestamp = 7731067; // Friday, 10 May 2019 6:04:05 diff --git a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java index df711f81a39..3b85d89e219 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java @@ -40,7 +40,7 @@ public class BlockHeaderTest { @Test public void getHashForMergedMiningWithForkDetectionDataAndIncludedOnAndMergedMiningFields() { - BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], true); + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], true, new byte[0]); byte[] encodedBlock = header.getEncoded(false, false); byte[] hashForMergedMining = Arrays.copyOfRange(HashUtil.keccak256(encodedBlock), 0, 20); @@ -56,7 +56,7 @@ public void getHashForMergedMiningWithForkDetectionDataAndIncludedOnAndMergedMin @Test public void getHashForMergedMiningWithNoForkDetectionDataAndIncludedOffAndMergedMiningFields() { - BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false); + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false, new byte[0]); Keccak256 hash = header.getHash(); byte[] hashForMergedMining = header.getHashForMergedMining(); @@ -67,7 +67,7 @@ public void getHashForMergedMiningWithNoForkDetectionDataAndIncludedOffAndMerged @Test public void getHashForMergedMiningWithForkDetectionDataAndIncludedOn() { byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeaderWithNoMergedMiningFields(forkDetectionData, true); + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(forkDetectionData, true, new byte[0]); byte[] hash = header.getHash().getBytes(); byte[] hashFirst20Elements = Arrays.copyOfRange(hash, 0, 20); @@ -81,7 +81,7 @@ public void getHashForMergedMiningWithForkDetectionDataAndIncludedOn() { @Test public void getHashForMergedMiningWithForkDetectionDataAndIncludedOff() { byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; - BlockHeader header = createBlockHeaderWithNoMergedMiningFields(forkDetectionData, false); + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(forkDetectionData, false, new byte[0]); byte[] hash = header.getHash().getBytes(); byte[] hashForMergedMining = header.getHashForMergedMining(); @@ -89,9 +89,32 @@ public void getHashForMergedMiningWithForkDetectionDataAndIncludedOff() { assertThat(hash, is(hashForMergedMining)); } + + @Test + public void getEncodedWithUmmRootWithMergedMiningFields() { + byte[] ummRoot = TestUtils.randomBytes(20); + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false, ummRoot); + + byte[] headerEncoded = header.getFullEncoded(); + RLPList headerRLP = RLP.decodeList(headerEncoded); + + assertThat(headerRLP.size(), is(20)); + } + + @Test + public void getEncodedWithUmmRootWithoutMergedMiningFields() { + byte[] ummRoot = TestUtils.randomBytes(20); + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(new byte[0], false, ummRoot); + + byte[] headerEncoded = header.getFullEncoded(); + RLPList headerRLP = RLP.decodeList(headerEncoded); + + assertThat(headerRLP.size(), is(17)); + } + @Test - public void getEncodedWithMergedMiningFields() { - BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false); + public void getEncodedNullUmmRootWithMergedMiningFields() { + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false, null); byte[] headerEncoded = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(headerEncoded); @@ -100,8 +123,8 @@ public void getEncodedWithMergedMiningFields() { } @Test - public void getEncodedWithoutMergedMiningFields() { - BlockHeader header = createBlockHeaderWithNoMergedMiningFields(new byte[0], false); + public void getEncodedNullUmmRootWithoutMergedMiningFields() { + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(new byte[0], false, null); byte[] headerEncoded = header.getFullEncoded(); RLPList headerRLP = RLP.decodeList(headerEncoded); @@ -109,9 +132,29 @@ public void getEncodedWithoutMergedMiningFields() { assertThat(headerRLP.size(), is(16)); } + @Test + public void getEncodedEmptyUmmRootWithMergedMiningFields() { + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false, new byte[0]); + + byte[] headerEncoded = header.getFullEncoded(); + RLPList headerRLP = RLP.decodeList(headerEncoded); + + assertThat(headerRLP.size(), is(20)); + } + + @Test + public void getEncodedEmptyUmmRootWithoutMergedMiningFields() { + BlockHeader header = createBlockHeaderWithNoMergedMiningFields(new byte[0], false, new byte[0]); + + byte[] headerEncoded = header.getFullEncoded(); + RLPList headerRLP = RLP.decodeList(headerEncoded); + + assertThat(headerRLP.size(), is(17)); + } + @Test public void getMiningForkDetectionData() { - BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], true); + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], true, new byte[0]); byte[] encodedBlock = header.getEncoded(false, false); byte[] hashForMergedMining = Arrays.copyOfRange(HashUtil.keccak256(encodedBlock), 0, 20); @@ -194,7 +237,7 @@ public void getHashForMergedMiningWhenUmmRootWithLengthOver20() { */ @Test(expected = IllegalStateException.class) public void getMiningForkDetectionDataNoDataCanBeFound() { - BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], true); + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], true, new byte[0]); byte[] forkDetectionData = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12 }; byte[] coinbase = concatenate(RskMiningConstants.RSK_TAG, forkDetectionData); @@ -206,23 +249,23 @@ public void getMiningForkDetectionDataNoDataCanBeFound() { @Test public void getMiningForkDetectionDataNoDataMustBeIncluded() { - BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false); + BlockHeader header = createBlockHeaderWithMergedMiningFields(new byte[0], false, new byte[0]); assertThat(new byte[0], is(header.getMiningForkDetectionData())); } private BlockHeader createBlockHeaderWithMergedMiningFields( byte[] forkDetectionData, - boolean includeForkDetectionData){ + boolean includeForkDetectionData, byte[] ummRoot){ return createBlockHeader(new byte[80], new byte[32], new byte[128], - forkDetectionData, includeForkDetectionData, null, false); + forkDetectionData, includeForkDetectionData, ummRoot, false); } private BlockHeader createBlockHeaderWithNoMergedMiningFields( byte[] forkDetectionData, - boolean includeForkDetectionData) { + boolean includeForkDetectionData, byte[] ummRoot) { return createBlockHeader(null, null, null, - forkDetectionData, includeForkDetectionData, null, true); + forkDetectionData, includeForkDetectionData, ummRoot, true); } private BlockHeader createBlockHeaderWithUmmRoot(byte[] ummRoot) { diff --git a/rskj-core/src/test/java/co/rsk/core/BlockTest.java b/rskj-core/src/test/java/co/rsk/core/BlockTest.java index cd5a3d47619..ddb8be31353 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockTest.java @@ -88,6 +88,7 @@ public void testParseRemascTransaction() { .setEmptyMergedMiningForkDetectionData() .setMinimumGasPrice(new Coin(BigInteger.TEN)) .setUncleCount(0) + .setUmmRoot(new byte[0]) .build(); Block block = blockFactory.newBlock( diff --git a/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java b/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java index a66a9adae97..22d3fe1998f 100644 --- a/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java +++ b/rskj-core/src/test/java/co/rsk/mine/BlockToMineBuilderTest.java @@ -88,7 +88,7 @@ public void setUp() { new ForkDetectionDataCalculator(), validationRules, mock(MinerClock.class), - new BlockFactory(ActivationConfigsForTest.all()), + new BlockFactory(activationConfig), blockExecutor, minimumGasPriceCalculator, minerUtils @@ -158,7 +158,7 @@ public void buildBlockBeforeUMMActivation() { blockBuilder.build(new ArrayList<>(Collections.singletonList(parent)), new byte[0]); Block actualBlock = blockCaptor.getValue(); - assertThat(actualBlock.getHeader().getUmmRoot(), is(new byte[0])); + assertNull(actualBlock.getHeader().getUmmRoot()); } @Test diff --git a/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java b/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java index 0123d61c8ed..66579391cbb 100644 --- a/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java +++ b/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java @@ -24,6 +24,7 @@ import co.rsk.crypto.Keccak256; import org.ethereum.TestUtils; import org.ethereum.config.blockchain.upgrades.ActivationConfigsForTest; +import org.ethereum.config.blockchain.upgrades.ConsensusRule; import org.ethereum.crypto.HashUtil; import org.ethereum.util.RLP; import org.ethereum.util.RLPList; @@ -263,7 +264,7 @@ public void createsHeaderWithUseRRSKIP92EncodingOn() { .build(); RLPList rlpList = RLP.decodeList(header.getEncoded()); - assertEquals(17, rlpList.size()); + assertEquals(18, rlpList.size()); } @Test @@ -281,7 +282,7 @@ public void createsHeaderWithUseRRSKIP92EncodingOff() { .build(); RLPList rlpList = RLP.decodeList(header.getEncoded()); - assertEquals(19, rlpList.size()); + assertEquals(20, rlpList.size()); } @Test @@ -300,7 +301,7 @@ public void createsHeaderWithUseRRSKIP92EncodingOffButConsensusCompliantOn() { // the useRskip92Field should be true, hence the merkle proof and coinbase are not included RLPList rlpList = RLP.decodeList(header.getEncoded()); - assertEquals(17, rlpList.size()); + assertEquals(18, rlpList.size()); } @Test @@ -371,9 +372,17 @@ public void createsHeaderWithUmmRoot() { } @Test - public void createsHeaderWithEmptyUmmRoot() { + public void createsHeaderWithEmptyUmmRootAndRskipUmmOn() { BlockHeader header = blockHeaderBuilder.build(); assertArrayEquals(new byte[0], header.getUmmRoot()); } + + @Test + public void createsHeaderWithEmptyUmmRootAndRskipUmmOff() { + BlockHeaderBuilder builder = new BlockHeaderBuilder(ActivationConfigsForTest.allBut(ConsensusRule.RSKIPUMM)); + BlockHeader header = builder.build(); + + assertNull(header.getUmmRoot()); + } } From e2ba82767c738c486af52b91a45baf8307946e52 Mon Sep 17 00:00:00 2001 From: mpicco Date: Mon, 9 Mar 2020 17:17:37 -0300 Subject: [PATCH 5/7] Truncate hashForMergedMining to a 20 byte array (UMM_LEAVE_LENGTH) before calculating the new hashForMergedMining by creating a merkle tree with the umm root --- rskj-core/src/main/java/org/ethereum/core/BlockHeader.java | 7 ++++--- rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java index 217e1f86a95..96342b05050 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java @@ -47,7 +47,7 @@ public class BlockHeader { private static final int HASH_FOR_MERGED_MINING_PREFIX_LENGTH = 20; private static final int FORK_DETECTION_DATA_LENGTH = 12; - private static final int UMM_ROOT_LENGTH = 20; + private static final int UMM_LEAVES_LENGTH = 20; /* The SHA3 256-bit hash of the parent block, in its entirety */ private byte[] parentHash; @@ -494,7 +494,8 @@ public byte[] getHashForMergedMining() { byte[] hashForMergedMining = HashUtil.keccak256(encodedBlock); if (isUMMBlock()) { - hashForMergedMining = this.getHashRootForMergedMining(hashForMergedMining); + byte[] leftHash = Arrays.copyOf(hashForMergedMining, UMM_LEAVES_LENGTH); + hashForMergedMining = this.getHashRootForMergedMining(leftHash); } if (includeForkDetectionData) { @@ -514,7 +515,7 @@ public byte[] getHashForMergedMining() { } private byte[] getHashRootForMergedMining(byte[] leftHash) { - if ((ummRoot.length != UMM_ROOT_LENGTH) && (ummRoot.length != 0)){ + if ((ummRoot.length != UMM_LEAVES_LENGTH) && (ummRoot.length != 0)){ throw new IllegalStateException( String.format("UMM Root length must be either 0 or 20. Found: %d", ummRoot.length) ); diff --git a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java index 3b85d89e219..9e6fc5834e2 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java @@ -207,7 +207,8 @@ public void getHashForMergedMiningWhenUmmRoot() { byte[] encodedBlock = header.getEncoded(false, false); byte[] oldHashForMergedMining = HashUtil.keccak256(encodedBlock); - byte[] hashRoot = HashUtil.keccak256(concatenate(oldHashForMergedMining, ummRoot)); + byte[] leftHash = Arrays.copyOf(oldHashForMergedMining, 20); + byte[] hashRoot = HashUtil.keccak256(concatenate(leftHash, ummRoot)); byte[] newHashForMergedMining = concatenate( java.util.Arrays.copyOfRange(hashRoot, 0, 20), forkDetectionData From 73d1d9d674d7e315216f8b018d797d41ff3c9c75 Mon Sep 17 00:00:00 2001 From: mpicco Date: Tue, 10 Mar 2020 15:40:13 -0300 Subject: [PATCH 6/7] Try making BlockFactory::decodeHeader a bit more flexible by avoiding checks for exact sizes --- .../java/org/ethereum/core/BlockFactory.java | 59 +++++++++++-------- .../org/ethereum/core/BlockHeaderBuilder.java | 15 ++++- .../rsk/blockchain/utils/BlockGenerator.java | 27 ++++++++- .../java/co/rsk/core/BlockFactoryTest.java | 16 +++-- .../java/co/rsk/net/messages/MessageTest.java | 32 ++++++---- .../ethereum/core/BlockHeaderBuilderTest.java | 44 ++++++++++++++ 6 files changed, 146 insertions(+), 47 deletions(-) diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java index 99978b994df..f6e6ae3eb5e 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockFactory.java @@ -38,11 +38,8 @@ import static org.ethereum.crypto.HashUtil.EMPTY_TRIE_HASH; public class BlockFactory { - private static final int RLP_HEADER_SIZE = 16; - private static final int RLP_HEADER_SIZE_WITH_MERGED_MINING = 19; - - private static final int RLP_HEADER_SIZE_UMM = 17; - private static final int RLP_HEADER_SIZE_UMM_WITH_MERGED_MINING = 20; + private static final int RLP_HEADER_SIZE = 17; + private static final int RLP_HEADER_SIZE_WITH_MERGED_MINING = 20; private final ActivationConfig activationConfig; @@ -100,13 +97,6 @@ public BlockHeader decodeHeader(byte[] encoded) { } private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { - if (!canBeDecoded(rlpHeader)) { - throw new IllegalArgumentException(String.format( - "A block header must have 16/17 elements or 19/20 including merged-mining fields but it had %d", - rlpHeader.size() - )); - } - byte[] parentHash = rlpHeader.get(0).getRLPData(); byte[] unclesHash = rlpHeader.get(1).getRLPData(); byte[] coinBaseBytes = rlpHeader.get(2).getRLPData(); @@ -135,7 +125,7 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { byte[] guBytes = rlpHeader.get(10).getRLPData(); byte[] tsBytes = rlpHeader.get(11).getRLPData(); - long number = parseBigInteger(nrBytes).longValueExact(); + long blockNumber = parseBigInteger(nrBytes).longValueExact(); long gasUsed = parseBigInteger(guBytes).longValueExact(); long timestamp = parseBigInteger(tsBytes).longValueExact(); @@ -146,14 +136,29 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { byte[] minimumGasPriceBytes = rlpHeader.get(14).getRLPData(); Coin minimumGasPrice = RLP.parseSignedCoinNonNullZero(minimumGasPriceBytes); + if (!canBeDecoded(rlpHeader, blockNumber)) { + throw new IllegalArgumentException(String.format( + "A block header must have 16/17 elements or 19/20 including merged-mining fields but it had %d", + rlpHeader.size() + )); + } + int r = 15; - byte[] ucBytes = rlpHeader.get(r++).getRLPData(); - int uncleCount = parseBigInteger(ucBytes).intValueExact(); + boolean isUmm = activationConfig.isActive(ConsensusRule.RSKIPUMM, blockNumber); + + boolean includeUncleCount = isUmm || + // sizes prior to UMM activation + rlpHeader.size() == (RLP_HEADER_SIZE-1) || rlpHeader.size() == (RLP_HEADER_SIZE_WITH_MERGED_MINING-1); + + int uncleCount = 0; + if (includeUncleCount) { + byte[] ucBytes = rlpHeader.get(r++).getRLPData(); + uncleCount = parseBigInteger(ucBytes).intValueExact(); + } byte[] ummRoot = null; - if (activationConfig.isActive(ConsensusRule.RSKIPUMM, number) && (rlpHeader.size() == RLP_HEADER_SIZE_UMM || - rlpHeader.size() == RLP_HEADER_SIZE_UMM_WITH_MERGED_MINING)) { + if (isUmm) { ummRoot = rlpHeader.get(r++).getRLPRawData(); } @@ -166,17 +171,17 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { bitcoinMergedMiningCoinbaseTransaction = rlpHeader.get(r++).getRLPData(); } - boolean useRskip92Encoding = activationConfig.isActive(ConsensusRule.RSKIP92, number); - boolean includeForkDetectionData = activationConfig.isActive(ConsensusRule.RSKIP110, number) && - number >= MiningConfig.REQUIRED_NUMBER_OF_BLOCKS_FOR_FORK_DETECTION_CALCULATION; + boolean useRskip92Encoding = activationConfig.isActive(ConsensusRule.RSKIP92, blockNumber); + boolean includeForkDetectionData = activationConfig.isActive(ConsensusRule.RSKIP110, blockNumber) && + blockNumber >= MiningConfig.REQUIRED_NUMBER_OF_BLOCKS_FOR_FORK_DETECTION_CALCULATION; - if (number == Genesis.NUMBER) { + if (blockNumber == Genesis.NUMBER) { return new GenesisHeader( parentHash, unclesHash, logsBloom, difficultyBytes, - number, + blockNumber, glBytes, gasUsed, timestamp, @@ -193,7 +198,7 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { return new BlockHeader( parentHash, unclesHash, coinbase, stateRoot, txTrieRoot, receiptTrieRoot, logsBloom, difficulty, - number, glBytes, gasUsed, timestamp, extraData, + blockNumber, glBytes, gasUsed, timestamp, extraData, paidFees, bitcoinMergedMiningHeader, bitcoinMergedMiningMerkleProof, bitcoinMergedMiningCoinbaseTransaction, new byte[0], minimumGasPrice, uncleCount, sealed, useRskip92Encoding, includeForkDetectionData, @@ -201,9 +206,11 @@ private BlockHeader decodeHeader(RLPList rlpHeader, boolean sealed) { ); } - private boolean canBeDecoded(RLPList rlpHeader) { - return rlpHeader.size() == RLP_HEADER_SIZE || rlpHeader.size() == RLP_HEADER_SIZE_WITH_MERGED_MINING || - rlpHeader.size() == RLP_HEADER_SIZE_UMM || rlpHeader.size() == RLP_HEADER_SIZE_UMM_WITH_MERGED_MINING; + private boolean canBeDecoded(RLPList rlpHeader, long blockNumber) { + int preUmmHeaderSizeAdjustment = activationConfig.isActive(ConsensusRule.RSKIPUMM, blockNumber) ? 0 : 1; + + return rlpHeader.size() == (RLP_HEADER_SIZE - preUmmHeaderSizeAdjustment) || + rlpHeader.size() == (RLP_HEADER_SIZE_WITH_MERGED_MINING - preUmmHeaderSizeAdjustment); } private static BigInteger parseBigInteger(byte[] bytes) { diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java index da2c03428fc..ac62a6a600f 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeaderBuilder.java @@ -67,10 +67,12 @@ public class BlockHeaderBuilder { private final ActivationConfig activationConfig; private boolean createConsensusCompliantHeader; + private boolean createUmmCompliantHeader; public BlockHeaderBuilder(ActivationConfig activationConfig) { this.activationConfig = activationConfig; createConsensusCompliantHeader = true; + createUmmCompliantHeader = true; } public BlockHeaderBuilder setCreateConsensusCompliantHeader(boolean createConsensusCompliantHeader) { @@ -78,6 +80,11 @@ public BlockHeaderBuilder setCreateConsensusCompliantHeader(boolean createConsen return this; } + public BlockHeaderBuilder setCreateUmmCompliantHeader(boolean createUmmCompliantHeader) { + this.createUmmCompliantHeader = createUmmCompliantHeader; + return this; + } + public BlockHeaderBuilder setStateRoot(byte[] stateRoot) { this.stateRoot = copy(stateRoot); return this; @@ -294,9 +301,11 @@ public BlockHeader build() { mergedMiningForkDetectionData.length > 0; } - if (activationConfig.isActive(ConsensusRule.RSKIPUMM, number)) { - if (ummRoot == null) { - ummRoot = new byte[0]; + if (createUmmCompliantHeader) { + if (activationConfig.isActive(ConsensusRule.RSKIPUMM, number)) { + if (ummRoot == null) { + ummRoot = new byte[0]; + } } } diff --git a/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java b/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java index 485571fa46e..6b40bc3a6c4 100644 --- a/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java +++ b/rskj-core/src/test/java/co/rsk/blockchain/utils/BlockGenerator.java @@ -56,7 +56,7 @@ public class BlockGenerator { private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; - private static final Block[] blockCache = new Block[5]; + private final Block[] blockCache = new Block[5]; private final DifficultyCalculator difficultyCalculator; private final BlockFactory blockFactory; @@ -148,6 +148,10 @@ public Block createChildBlock(Block parent) { public Block createChildBlock(Block parent, long fees, List uncles, byte[] difficulty) { byte[] unclesListHash = HashUtil.keccak256(BlockHeader.getUnclesEncodedEx(uncles)); + long blockNumber = parent.getNumber() + 1; + + byte[] ummRoot = activationConfig.isActive(ConsensusRule.RSKIPUMM, blockNumber) ? new byte[0] : null; + BlockHeader newHeader = blockFactory.getBlockHeaderBuilder() .setParentHashFromKeccak256(parent.getHash()) .setUnclesHash(unclesListHash).setCoinbase(parent.getCoinbase()) @@ -162,7 +166,7 @@ public Block createChildBlock(Block parent, long fees, List uncles, .setPaidFees(Coin.valueOf(fees)) .setEmptyMergedMiningForkDetectionData() .setUncleCount(uncles.size()) - .setUmmRoot(new byte[0]) + .setUmmRoot(ummRoot) .build(); return blockFactory.newBlock( @@ -181,6 +185,10 @@ public Block createChildBlock(Block parent, List txs, byte[] stateR boolean isRskip126Enabled = activationConfig.isActive(ConsensusRule.RSKIP126, 0); + long blockNumber = parent.getNumber() + 1; + + byte[] ummRoot = activationConfig.isActive(ConsensusRule.RSKIPUMM, blockNumber) ? new byte[0] : null; + BlockHeader newHeader = blockFactory.getBlockHeaderBuilder() .setParentHashFromKeccak256(parent.getHash()) .setTxTrieRoot(BlockHashesHelper.getTxTrieRoot(txs, isRskip126Enabled)) @@ -194,6 +202,7 @@ public Block createChildBlock(Block parent, List txs, byte[] stateR .setGasUsed( parent.getGasUsed()) // why ? I just copied the value before .setTimestamp(parent.getTimestamp() + ++count) .setEmptyMergedMiningForkDetectionData() + .setUmmRoot(ummRoot) .build(); return blockFactory.newBlock( @@ -246,6 +255,10 @@ public Block createChildBlock(Block parent, List txs, List txs, List headers = new ArrayList<>(); for (int k = 1; k <= 4; k++) - headers.add(new BlockGenerator().getBlock(k).getHeader()); + headers.add(blockGenerator.getBlock(k).getHeader()); BlockHeadersResponseMessage message = new BlockHeadersResponseMessage(100, headers); @@ -218,7 +226,7 @@ public void encodeDecodeBlockHeadersResponseMessage() { @Test public void encodeDecodeNewBlockHashesMessage() { - List blocks = new BlockGenerator().getBlockChain(10); + List blocks = blockGenerator.getBlockChain(10); Block b1 = blocks.get(5); Block b2 = blocks.get(7); @@ -360,7 +368,7 @@ public void encodeDecodeBlockHeadersRequestMessage() { @Test public void encodeDecodeSkeletonResponseMessage() { long someId = 42; - List blocks = new BlockGenerator().getBlockChain(10); + List blocks = blockGenerator.getBlockChain(10); Block b1 = blocks.get(5); Block b2 = blocks.get(7); @@ -432,7 +440,7 @@ public void encodeDecodeNewBlockHashMessage() { @Test public void encodeDecodeBodyRequestMessage() { - Block block = new BlockGenerator().getBlock(1); + Block block = blockGenerator.getBlock(1); BodyRequestMessage message = new BodyRequestMessage(100, block.getHash().getBytes()); byte[] encoded = message.getEncoded(); @@ -458,7 +466,7 @@ public void encodeDecodeBodyResponseMessage() { List uncles = new ArrayList<>(); - BlockGenerator blockGenerator = new BlockGenerator(); + BlockGenerator blockGenerator = this.blockGenerator; Block parent = blockGenerator.getGenesisBlock(); for (int k = 1; k < 10; k++) { diff --git a/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java b/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java index 66579391cbb..afd832da5e1 100644 --- a/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java +++ b/rskj-core/src/test/java/org/ethereum/core/BlockHeaderBuilderTest.java @@ -385,4 +385,48 @@ public void createsHeaderWithEmptyUmmRootAndRskipUmmOff() { assertNull(header.getUmmRoot()); } + + @Test + public void createsHeaderWithNullUmmrootButUmmCompliantHeaderOn() { + BlockHeader header = blockHeaderBuilder + .setCreateUmmCompliantHeader(true) + .setUmmRoot(null) + .build(); + + assertArrayEquals(new byte[0], header.getUmmRoot()); + } + + @Test + public void createsHeaderWithNullUmmrootButUmmCompliantHeaderOff() { + BlockHeader header = blockHeaderBuilder + .setCreateUmmCompliantHeader(false) + .setUmmRoot(null) + .build(); + + assertArrayEquals(null, header.getUmmRoot()); + } + + @Test + public void createsHeaderWithNullUmmrootButUmmCompliantHeaderOnAndRskipUmmOff() { + BlockHeaderBuilder builder = new BlockHeaderBuilder(ActivationConfigsForTest.allBut(ConsensusRule.RSKIPUMM)); + + BlockHeader header = builder + .setCreateUmmCompliantHeader(true) + .setUmmRoot(null) + .build(); + + assertNull(header.getUmmRoot()); + } + + @Test + public void createsHeaderWithNullUmmrootButUmmCompliantHeaderOffAndRskipUmmOff() { + BlockHeaderBuilder builder = new BlockHeaderBuilder(ActivationConfigsForTest.allBut(ConsensusRule.RSKIPUMM)); + + BlockHeader header = builder + .setCreateUmmCompliantHeader(false) + .setUmmRoot(null) + .build(); + + assertArrayEquals(null, header.getUmmRoot()); + } } From 52fca5c5fa2bdd2a715022a8c5efad9982486b27 Mon Sep 17 00:00:00 2001 From: mpicco Date: Wed, 11 Mar 2020 15:35:57 -0300 Subject: [PATCH 7/7] Remove unnecesary ummRoot.length!=0 check since it was already done outside the invoked method --- .../src/main/java/org/ethereum/core/BlockHeader.java | 2 +- .../src/test/java/co/rsk/core/BlockHeaderTest.java | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java index 96342b05050..fd163495204 100644 --- a/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java +++ b/rskj-core/src/main/java/org/ethereum/core/BlockHeader.java @@ -515,7 +515,7 @@ public byte[] getHashForMergedMining() { } private byte[] getHashRootForMergedMining(byte[] leftHash) { - if ((ummRoot.length != UMM_LEAVES_LENGTH) && (ummRoot.length != 0)){ + if (ummRoot.length != UMM_LEAVES_LENGTH){ throw new IllegalStateException( String.format("UMM Root length must be either 0 or 20. Found: %d", ummRoot.length) ); diff --git a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java index 9e6fc5834e2..a704efe3592 100644 --- a/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java +++ b/rskj-core/src/test/java/co/rsk/core/BlockHeaderTest.java @@ -217,6 +217,18 @@ public void getHashForMergedMiningWhenUmmRoot() { assertThat(header.getHashForMergedMining(), is(newHashForMergedMining)); } + @Test + public void getHashForMergedMiningWhenEmptyUmmRoot() { + byte[] ummRoot = new byte[0]; + + BlockHeader header = createBlockHeaderWithUmmRoot(ummRoot); + + byte[] encodedBlock = header.getEncoded(false, false); + byte[] hashForMergedMining = HashUtil.keccak256(encodedBlock); + + assertThat(header.getHashForMergedMining(), is(hashForMergedMining)); + } + @Test(expected = IllegalStateException.class) public void getHashForMergedMiningWhenUmmRootWithLengthUnder20() { byte[] ummRoot = new byte[]{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18};