diff --git a/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProvider.java b/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProvider.java index c27eb9afd..cf747c963 100644 --- a/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProvider.java +++ b/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProvider.java @@ -34,15 +34,12 @@ public ConfirmedBlocksProvider( } public List getConfirmedBlocks(Keccak256 startingPoint) { - List potentialBlocks = new ArrayList<>(); - List confirmedBlocks = new ArrayList<>(); - BigInteger accumulatedDifficulty = BigInteger.ZERO; - Block initialBlock = blockStore.getBlockByHash(startingPoint.getBytes()); + long initialBlockNumber = initialBlock.getNumber(); Block bestBlock = blockStore.getBestBlock(); logger.trace( "[getConfirmedBlocks] Initial block height is {} and RSK best block height {}. Using HSM version {}, difficulty target {}, difficulty cap {}, sending max {} elements", - initialBlock.getNumber(), + initialBlockNumber, bestBlock.getNumber(), hsmVersion, minimumAccumulatedDifficulty, @@ -50,27 +47,33 @@ public List getConfirmedBlocks(Keccak256 startingPoint) { maximumElementsToSendHSM ); - int lastIndexToConfirmBlock = 0; - Block blockToProcess = blockStore.getChainBlockByNumber(initialBlock.getNumber() + 1); + List blocksInWindow = new ArrayList<>(); + int proofBlocksCount = 0; + BigInteger accumulatedDifficulty = BigInteger.ZERO; + List confirmedBlocks = new ArrayList<>(); + + Block blockToProcess = blockStore.getChainBlockByNumber(initialBlockNumber + 1); while (blockToProcess != null && confirmedBlocks.size() < maximumElementsToSendHSM) { - potentialBlocks.add(blockToProcess); - BigInteger difficultyToConsider = getBlockDifficultyToConsider(blockToProcess); - accumulatedDifficulty = accumulatedDifficulty.add(difficultyToConsider); + blocksInWindow.add(blockToProcess); + BigInteger totalDifficulty = getBlockTotalDifficulty(blockToProcess, initialBlockNumber); + accumulatedDifficulty = accumulatedDifficulty.add(totalDifficulty); - if (accumulatedDifficulty.compareTo(minimumAccumulatedDifficulty) >= 0) { // Enough difficulty accumulated + boolean enoughDifficulty = accumulatedDifficulty.compareTo(minimumAccumulatedDifficulty) >= 0; + if (enoughDifficulty) { logger.trace( "[getConfirmedBlocks] Accumulated enough difficulty {} with {} blocks", accumulatedDifficulty, - potentialBlocks.size() + blocksInWindow.size() ); - // The first block was confirmed. Add it to confirm, subtract its difficulty from the accumulated and from the potentials list - Block confirmedBlock = potentialBlocks.get(0); + // The block was confirmed. Add it to confirmed blocks list, + // subtract its difficulty from the accumulated and remove it from the proof blocks list + Block confirmedBlock = blocksInWindow.get(0); confirmedBlocks.add(confirmedBlock); - BigInteger confirmedBlockDifficultyToConsider = getBlockDifficultyToConsider(confirmedBlock); - accumulatedDifficulty = accumulatedDifficulty.subtract(confirmedBlockDifficultyToConsider); - potentialBlocks.remove(confirmedBlock); - lastIndexToConfirmBlock = potentialBlocks.size(); + BigInteger confirmedBlockTotalDifficulty = getBlockTotalDifficulty(confirmedBlock, initialBlockNumber); + accumulatedDifficulty = accumulatedDifficulty.subtract(confirmedBlockTotalDifficulty); + blocksInWindow.remove(confirmedBlock); + proofBlocksCount = blocksInWindow.size(); logger.trace( "[getConfirmedBlocks] Confirmed block {} (height {})", @@ -85,33 +88,30 @@ public List getConfirmedBlocks(Keccak256 startingPoint) { if (confirmedBlocks.isEmpty()) { return confirmedBlocks; } - // Adding the proof of the confirmed elements from the potential elements - potentialBlocks = potentialBlocks.subList(0, lastIndexToConfirmBlock); - confirmedBlocks.addAll(potentialBlocks); - logger.debug("[getConfirmedBlocks] Added {} extra blocks as proof", potentialBlocks.size()); + // Adding the proof of the confirmed elements from the blocks remaining in the window + blocksInWindow = blocksInWindow.subList(0, proofBlocksCount); + confirmedBlocks.addAll(blocksInWindow); + logger.debug("[getConfirmedBlocks] Added {} extra blocks as proof", blocksInWindow.size()); return confirmedBlocks; } - protected BigInteger getBlockDifficultyToConsider(Block block) { + protected BigInteger getBlockTotalDifficulty(Block block, long uncleHeightThreshold) { logger.trace( - "[getBlockDifficultyToConsider] Get difficulty for block {} at height {}", + "[getBlockTotalDifficulty] Get total difficulty for block {} at height {}", block.getHash(), block.getNumber() ); - BigInteger blockTotalDifficulty = block.getDifficulty().asBigInteger(); - - BigInteger blockDifficultyToConsider = difficultyCap.min(blockTotalDifficulty); - BigInteger unclesDifficultyToConsider = block.getUncleList().stream() + BigInteger blockDifficulty = difficultyCap.min(block.getDifficulty().asBigInteger()); + // Each block uncle is sent to the HSM as a brother of the respective canonical block + // it shares a parent with, which is part of the set being sent only when the + // original block's uncle is above the HSM best block. + // So only those uncles can be delivered as brothers and counted. + BigInteger unclesDifficulty = block.getUncleList().stream() + .filter(uncle -> uncle.getNumber() > uncleHeightThreshold) .map(uncle -> difficultyCap.min(uncle.getDifficulty().asBigInteger())) .reduce(BigInteger.ZERO, BigInteger::add); - - logger.trace( - "[getBlockDifficultyToConsider] Block difficulty {}, considering {}", - blockTotalDifficulty, - blockDifficultyToConsider - ); - return blockDifficultyToConsider.add(unclesDifficultyToConsider); + return blockDifficulty.add(unclesDifficulty); } } diff --git a/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HSMBookkeepingService.java b/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HSMBookkeepingService.java index d06a75240..6265cf5d1 100644 --- a/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HSMBookkeepingService.java +++ b/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HSMBookkeepingService.java @@ -154,24 +154,27 @@ protected void informConfirmedBlockHeaders() { hsmCurrentBestBlock.getNumber() ); - List blocks = this.confirmedBlocksProvider.getConfirmedBlocks(hsmCurrentBestBlock.getHash()); - if (blocks.isEmpty()) { + List confirmedBlocks = this.confirmedBlocksProvider.getConfirmedBlocks(hsmCurrentBestBlock.getHash()); + if (confirmedBlocks.isEmpty()) { logger.debug("[informConfirmedBlockHeaders] No new block headers to inform"); logger.info("[informConfirmedBlockHeaders] Finished HSM bookkeeping process"); informing = false; return; } + int confirmedBlocksSize = confirmedBlocks.size(); + Block firstConfirmedBlock = confirmedBlocks.get(0); + Block lastConfirmedBlock = confirmedBlocks.get(confirmedBlocksSize - 1); logger.debug( "[informConfirmedBlockHeaders] Going to inform {} block headers. From block number {} with hash {} to block number {} with hash {}", - blocks.size(), - blocks.get(0).getNumber(), - blocks.get(0).getHash(), - blocks.get(blocks.size() - 1).getNumber(), - blocks.get(blocks.size() - 1).getHash() + confirmedBlocksSize, + firstConfirmedBlock.getNumber(), + firstConfirmedBlock.getHash(), + lastConfirmedBlock.getNumber(), + lastConfirmedBlock.getHash() ); - hsmBookkeepingClient.advanceBlockchain(blocks); + hsmBookkeepingClient.advanceBlockchain(confirmedBlocks); hsmCurrentBestBlock = getHsmBestBlock(); logger.debug( "[informConfirmedBlockHeaders] HSM best block after informing {} (height: {})", diff --git a/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HsmBookkeepingClientImpl.java b/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HsmBookkeepingClientImpl.java index 5797c2223..8c665f631 100644 --- a/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HsmBookkeepingClientImpl.java +++ b/src/main/java/co/rsk/federate/signing/hsm/advanceblockchain/HsmBookkeepingClientImpl.java @@ -43,7 +43,7 @@ public class HsmBookkeepingClientImpl implements HSMBookkeepingClient { private static final Logger logger = LoggerFactory.getLogger(HsmBookkeepingClientImpl.class); private final HSMClientProtocol hsmClientProtocol; private final HSMVersion hsmVersion; - private int maxChunkSize = 10; // DEFAULT VALUE + private int maxChunkSize = 10; // DEFAULT VALUE private boolean isStopped = false; public HsmBookkeepingClientImpl(HSMClientProtocol hsmClientProtocol) throws HSMClientException { @@ -126,9 +126,12 @@ private void addBrothersToPayload(ObjectNode payload, List brothers) { payload.set(BROTHERS.getFieldName(), brothersFieldData); } - private List getBrothers(String[] blockHeaderChunk, AdvanceBlockchainMessage message) - throws HSMBlockchainBookkeepingRelatedException { + private List getBrothers( + String[] blockHeaderChunk, + AdvanceBlockchainMessage message + ) throws HSMBlockchainBookkeepingRelatedException { List brothers = new ArrayList<>(); + for (String blockHeader : blockHeaderChunk) { brothers.add(message.getParsedBrothers(blockHeader)); } @@ -163,16 +166,19 @@ public void updateAncestorBlock(UpdateAncestorBlockMessage updateAncestorBlockMe } @Override - public void advanceBlockchain(List blocks) throws HSMClientException { - AdvanceBlockchainMessage message = new AdvanceBlockchainMessage(blocks); + public void advanceBlockchain(List confirmedBlocks) throws HSMClientException { + String advanceBlockchain = ADVANCE_BLOCKCHAIN.getCommand(); + + AdvanceBlockchainMessage message = new AdvanceBlockchainMessage(confirmedBlocks); List blockHeaders = message.getParsedBlockHeaders(); - validateHSMStateAndBlockHeaders(blockHeaders, ADVANCE_BLOCKCHAIN.getCommand()); + validateHSMStateAndBlockHeaders(blockHeaders, advanceBlockchain); List blockHeadersChunks = getChunks(blockHeaders.toArray(new String[]{}), maxChunkSize, false); - logger.trace("[advanceBlockchain] Going to send {} headers in {} chunks.", blockHeaders.size(), blockHeadersChunks.size()); - for (int i = 0; i < blockHeadersChunks.size(); i++) { + int blockHeadersChunksSize = blockHeadersChunks.size(); + logger.trace("[advanceBlockchain] Going to send {} headers in {} chunks.", blockHeaders.size(), blockHeadersChunksSize); + for (int i = 0; i < blockHeadersChunksSize; i++) { String[] blockHeaderChunk = blockHeadersChunks.get(i); - ObjectNode payload = this.hsmClientProtocol.buildCommand(ADVANCE_BLOCKCHAIN.getCommand(), hsmVersion); + ObjectNode payload = this.hsmClientProtocol.buildCommand(advanceBlockchain, hsmVersion); addBlocksToPayload(payload, blockHeaderChunk); List brothers = getBrothers(blockHeaderChunk, message); diff --git a/src/main/java/co/rsk/federate/signing/hsm/message/AdvanceBlockchainMessage.java b/src/main/java/co/rsk/federate/signing/hsm/message/AdvanceBlockchainMessage.java index 12caf3439..1646c94a5 100644 --- a/src/main/java/co/rsk/federate/signing/hsm/message/AdvanceBlockchainMessage.java +++ b/src/main/java/co/rsk/federate/signing/hsm/message/AdvanceBlockchainMessage.java @@ -14,12 +14,12 @@ public class AdvanceBlockchainMessage { protected static final int BROTHERS_LIMIT_PER_BLOCK_HEADER = 10; private final List parsedHeaders; - public AdvanceBlockchainMessage(List blocks) { - this.parsedHeaders = parseHeadersAndBrothers(blocks); + public AdvanceBlockchainMessage(List confirmedBlocks) { + this.parsedHeaders = parseHeadersAndBrothers(confirmedBlocks); } public List getParsedBlockHeaders() { - return this.parsedHeaders.stream().map(ParsedHeader::getBlockHeader).collect(Collectors.toList()); + return this.parsedHeaders.stream().map(ParsedHeader::getBlockHeader).toList(); } public String[] getParsedBrothers(String blockHeader) throws HSMBlockchainBookkeepingRelatedException { @@ -27,35 +27,36 @@ public String[] getParsedBrothers(String blockHeader) throws HSMBlockchainBookke .filter(header -> header.getBlockHeader().equals(blockHeader)) .findFirst() .map(ParsedHeader::getBrothers) - .orElseThrow(() -> new HSMBlockchainBookkeepingRelatedException("Error while trying to get brothers for block header. Could not find header: " + blockHeader)); + .orElseThrow( + () -> new HSMBlockchainBookkeepingRelatedException("Error while trying to get brothers for block header. Could not find header " + blockHeader) + ); } - private List parseHeadersAndBrothers(List blocks) { - Map> brothersByParentHash = groupBrothersByParentHash(blocks); + private List parseHeadersAndBrothers(List confirmedBlocks) { + Map> brothersByParentHash = groupBrothersByParentHash(confirmedBlocks); - return blocks.stream() + return confirmedBlocks.stream() .sorted(Comparator.comparingLong(Block::getNumber).reversed()) // sort blocks from latest to oldest .map(block -> new ParsedHeader( block.getHeader(), - filterBrothers(brothersByParentHash.getOrDefault(block.getParentHash(), Collections.emptyList())) - )).collect(Collectors.toList()); + capAmountOfBrothers(brothersByParentHash.getOrDefault(block.getParentHash(), Collections.emptyList())) + )).toList(); } - private Map> groupBrothersByParentHash(List blocks) { - return blocks.stream() + private Map> groupBrothersByParentHash(List confirmedBlocks) { + return confirmedBlocks.stream() .skip(1) // Skip the oldest block (index 0) because its uncles doesn't belong to this set of blocks .flatMap(block -> block.getUncleList().stream()) .collect(Collectors.groupingBy(BlockHeader::getParentHash)); } - private List filterBrothers(List brothers) { + private List capAmountOfBrothers(List brothers) { if (brothers.size() <= BROTHERS_LIMIT_PER_BLOCK_HEADER) { return brothers; } return brothers.stream() - .sorted((brother1, brother2) -> - brother2.getDifficulty().asBigInteger().compareTo(brother1.getDifficulty().asBigInteger())) + .sorted((brother1, brother2) -> brother2.getDifficulty().compareTo(brother1.getDifficulty())) .limit(BROTHERS_LIMIT_PER_BLOCK_HEADER) - .collect(Collectors.toList()); + .toList(); } } diff --git a/src/test/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProviderTest.java b/src/test/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProviderTest.java index 2098d1cd2..7e0b8c430 100644 --- a/src/test/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProviderTest.java +++ b/src/test/java/co/rsk/federate/signing/hsm/advanceblockchain/ConfirmedBlocksProviderTest.java @@ -182,7 +182,7 @@ void getConfirmedBlocks_belowDifficultyCap_ok() { } @Test - void getBlockDifficultyToConsider_considersUnclesAndCapDifficulty() { + void getBlockTotalDifficulty_considersUnclesAndCapDifficulty() { // arrange Block block = buildBlockWithUncles(); @@ -196,7 +196,8 @@ void getBlockDifficultyToConsider_considersUnclesAndCapDifficulty() { ); // act - BigInteger consideredDifficulty = confirmedBlocksProvider.getBlockDifficultyToConsider(block); + int bestBlockHeight = 1; + BigInteger totalDifficulty = confirmedBlocksProvider.getBlockTotalDifficulty(block, bestBlockHeight); // assert // Pow HSM considers brothers difficulty @@ -204,8 +205,39 @@ void getBlockDifficultyToConsider_considersUnclesAndCapDifficulty() { // + 1000000000000000000000 difficulty from block 2 (uncle) // + 8000000000000000000000 difficulty round to 7000000000000000000000 from block 3 (uncle) // = 15000000000000000000000 considered difficulty - BigInteger expectedConsideredDifficulty = new BigInteger("15000000000000000000000"); - assertEquals(expectedConsideredDifficulty, consideredDifficulty); + BigInteger expectedTotalDifficulty = new BigInteger("15000000000000000000000"); + assertEquals(expectedTotalDifficulty, totalDifficulty); + } + + @Test + void getBlockTotalDifficulty_ignoresUnclesNotAboveHsmBestBlock() { + // arrange + Block block = buildBlockWithUncles(); + + // build blocks provider for Pow HSM + ConfirmedBlocksProvider confirmedBlocksProvider = new ConfirmedBlocksProvider( + BigInteger.valueOf(160), + 100, + mock(BlockStore.class), + MAINNET.getDifficultyCap(), + hsmVersion + ); + + // act + // With the HSM best block at height 2, the uncle at height 2 is a sibling of the HSM best + // block: its canonical sibling is not part of the set being sent, so it cannot be delivered + // as a brother and its difficulty must be ignored. Only the uncle at height 3 is counted. + int bestBlockHeight = 2; + BigInteger totalDifficulty = confirmedBlocksProvider.getBlockTotalDifficulty(block, bestBlockHeight); + + // assert + // Pow HSM considers brothers difficulty + // 7000000000000000000001 difficulty round to 7000000000000000000000 from block 4 + // + 8000000000000000000000 difficulty round to 7000000000000000000000 from block 3 (uncle) + // block 2 (uncle at height 2) is NOT counted because it cannot be sent as a brother + // = 14000000000000000000000 considered difficulty + BigInteger expectedTotalDifficulty = new BigInteger("14000000000000000000000"); + assertEquals(expectedTotalDifficulty, totalDifficulty); } private Block buildBlockWithUncles() {