From 547494bb82d960b5e6ce441fcc9aaf511fc6f367 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Mon, 16 Dec 2024 11:03:52 +0200 Subject: [PATCH 01/18] feat: add getThreshold() method for grandpa state --- .../com/limechain/grandpa/GrandpaService.java | 17 ++++++++++ .../limechain/grandpa/state/GrandpaState.java | 31 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 src/main/java/com/limechain/grandpa/GrandpaService.java create mode 100644 src/main/java/com/limechain/grandpa/state/GrandpaState.java diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java new file mode 100644 index 00000000..60609fe8 --- /dev/null +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -0,0 +1,17 @@ +package com.limechain.grandpa; + +import com.limechain.grandpa.state.GrandpaState; +import lombok.extern.java.Log; +import org.springframework.stereotype.Component; + +@Log +@Component +public class GrandpaService { + private final GrandpaState grandpaState; + + public GrandpaService(GrandpaState grandpaState) { + this.grandpaState = grandpaState; + } + + private void getGrandpaGhost(){} +} diff --git a/src/main/java/com/limechain/grandpa/state/GrandpaState.java b/src/main/java/com/limechain/grandpa/state/GrandpaState.java new file mode 100644 index 00000000..97f24c50 --- /dev/null +++ b/src/main/java/com/limechain/grandpa/state/GrandpaState.java @@ -0,0 +1,31 @@ +package com.limechain.grandpa.state; + +import com.limechain.chain.lightsyncstate.Authority; +import lombok.Getter; +import org.springframework.stereotype.Component; + +import java.math.BigInteger; +import java.util.List; + +/** + * Represents the state information for the current round and authorities that are needed + * for block finalization with GRANDPA. + * Note: Intended for use only when the host is configured as an Authoring Node. + */ +@Getter +@Component +public class GrandpaState { + + private static final BigInteger THRESHOLD_NUMERATOR = BigInteger.valueOf(2); + private static final BigInteger THRESHOLD_DENOMINATOR = BigInteger.valueOf(3); + + private List voters; + private BigInteger setId; + private BigInteger roundNumber; + + public BigInteger getThreshold() { + var votersCount = BigInteger.valueOf(voters.size()); + return THRESHOLD_NUMERATOR.multiply(votersCount) + .divide(THRESHOLD_DENOMINATOR); + } +} From 6f570c18c57b2cf41087bf4216bf5beb66203e44 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Mon, 16 Dec 2024 11:06:40 +0200 Subject: [PATCH 02/18] feat: add derivePrimary() to grandpa state --- src/main/java/com/limechain/grandpa/state/GrandpaState.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/com/limechain/grandpa/state/GrandpaState.java b/src/main/java/com/limechain/grandpa/state/GrandpaState.java index 97f24c50..50f17c73 100644 --- a/src/main/java/com/limechain/grandpa/state/GrandpaState.java +++ b/src/main/java/com/limechain/grandpa/state/GrandpaState.java @@ -28,4 +28,9 @@ public BigInteger getThreshold() { return THRESHOLD_NUMERATOR.multiply(votersCount) .divide(THRESHOLD_DENOMINATOR); } + + public BigInteger derivePrimary() { + var votersCount = BigInteger.valueOf(voters.size()); + return roundNumber.remainder(votersCount); + } } From b6728e1bc93f06395ed6adea311d511650e7c5d1 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Mon, 16 Dec 2024 11:38:45 +0200 Subject: [PATCH 03/18] feat: add subround enum --- src/main/java/com/limechain/grandpa/state/Subround.java | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/main/java/com/limechain/grandpa/state/Subround.java diff --git a/src/main/java/com/limechain/grandpa/state/Subround.java b/src/main/java/com/limechain/grandpa/state/Subround.java new file mode 100644 index 00000000..58e2a46e --- /dev/null +++ b/src/main/java/com/limechain/grandpa/state/Subround.java @@ -0,0 +1,9 @@ +package com.limechain.grandpa.state; + +public enum Subround { + PRE_VOTE(0), + PRE_COMMIT(1), + PRIMARY_PROPOSAL(2); + + Subround(int value) {} +} From 278925c427f8fc747c35d4e18159a8fb1d34e745 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Tue, 17 Dec 2024 18:05:06 +0200 Subject: [PATCH 04/18] feat: add auxiliary methods for grandpa ghost --- .../com/limechain/grandpa/GrandpaService.java | 107 +++++++++++++++++- .../limechain/grandpa/state/GrandpaState.java | 21 +++- 2 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index 60609fe8..a466aa72 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -1,17 +1,120 @@ package com.limechain.grandpa; import com.limechain.grandpa.state.GrandpaState; +import com.limechain.grandpa.state.Subround; +import com.limechain.network.protocol.grandpa.messages.commit.Vote; +import com.limechain.storage.block.BlockState; +import io.emeraldpay.polkaj.types.Hash256; import lombok.extern.java.Log; +import org.bouncycastle.math.ec.rfc8032.Ed25519; import org.springframework.stereotype.Component; +import java.math.BigInteger; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + @Log @Component public class GrandpaService { private final GrandpaState grandpaState; + private final BlockState blockState; - public GrandpaService(GrandpaState grandpaState) { + public GrandpaService(GrandpaState grandpaState, BlockState blockState) { this.grandpaState = grandpaState; + this.blockState = blockState; + } + + //TODO: Implement further + private void getGrandpaGhost(BigInteger roundNumber){ + var threshold = grandpaState.getThreshold(); + + } + + private HashMap getPossibleSelectedBlocks(Long threshold, Subround subround) { + var votes = getDirectVotes(subround); + var blocks = new HashMap(); + + for (Vote vote : votes.keySet()) { + long totalVotes = getTotalVotesForBlock(vote.getBlockHash(), subround); + if (totalVotes > threshold) { + blocks.put(vote.getBlockHash(), vote.getBlockNumber()); + } + } + + if (!blocks.isEmpty()) { + return blocks; + } + + List allVotes = getVotes(subround); + for (Vote vote : votes.keySet()) { + //TODO: Implement further +// blocks = getPossibleSelectedAncestors(allVotes, v.getHash(), blocks, stage, threshold); + } + + return blocks; + } + + private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { + long votesForBlock = getVotesForBlock(blockHash, subround); + + if (votesForBlock == 0L) { + return 0L; + } + + int equivocationCount = 0; + + switch (subround) { + case Subround.PRE_VOTE -> equivocationCount = grandpaState.getPvEquivocations().size(); + case Subround.PRE_COMMIT -> equivocationCount = grandpaState.getPcEquivocations().size(); + } + + return votesForBlock + equivocationCount; + } + + private long getVotesForBlock(Hash256 blockHash, Subround subround) { + var votes = getDirectVotes(subround); + long votesForBlock = 0L; + + for (Map.Entry entry : votes.entrySet()) { + Vote v = entry.getKey(); + long count = entry.getValue(); + + try { + // Check if the current block is a descendant of the given block + boolean isDescendant = blockState.isDescendantOf(blockHash, v.getBlockHash()); + if (isDescendant) { + votesForBlock += count; + } + + } catch (Exception e) { + //TODO: adjust the exception type + return 0L; + } + } + + return votesForBlock; } - private void getGrandpaGhost(){} + // returns a map of votes to direct vote count + private ConcurrentHashMap getDirectVotes(Subround subround) { + var votes = new ConcurrentHashMap(); + var voteCountMap = new ConcurrentHashMap(); + + switch (subround) { + case Subround.PRE_VOTE -> votes.putAll(grandpaState.getPrevotes()); + case Subround.PRE_COMMIT -> votes.putAll(grandpaState.getPrecommits()); + } + + votes.values().forEach(vote -> voteCountMap.merge(vote, 1L, Long::sum)); + + return voteCountMap; + } + + private List getVotes(Subround subround) { + var votes = getDirectVotes(subround); + return new ArrayList<>(votes.keySet()); + } } diff --git a/src/main/java/com/limechain/grandpa/state/GrandpaState.java b/src/main/java/com/limechain/grandpa/state/GrandpaState.java index 50f17c73..8401ce8f 100644 --- a/src/main/java/com/limechain/grandpa/state/GrandpaState.java +++ b/src/main/java/com/limechain/grandpa/state/GrandpaState.java @@ -1,11 +1,16 @@ package com.limechain.grandpa.state; import com.limechain.chain.lightsyncstate.Authority; +import com.limechain.network.protocol.grandpa.messages.catchup.res.SignedVote; +import com.limechain.network.protocol.grandpa.messages.commit.Vote; import lombok.Getter; +import org.bouncycastle.math.ec.rfc8032.Ed25519; import org.springframework.stereotype.Component; import java.math.BigInteger; import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Represents the state information for the current round and authorities that are needed @@ -16,17 +21,21 @@ @Component public class GrandpaState { - private static final BigInteger THRESHOLD_NUMERATOR = BigInteger.valueOf(2); - private static final BigInteger THRESHOLD_DENOMINATOR = BigInteger.valueOf(3); + private static final long THRESHOLD_NUMERATOR = 2L; + private static final long THRESHOLD_DENOMINATOR = 3L; private List voters; private BigInteger setId; private BigInteger roundNumber; - public BigInteger getThreshold() { - var votersCount = BigInteger.valueOf(voters.size()); - return THRESHOLD_NUMERATOR.multiply(votersCount) - .divide(THRESHOLD_DENOMINATOR); + //TODO: This may not be the best place for those maps + private Map precommits = new ConcurrentHashMap<>(); + private Map prevotes = new ConcurrentHashMap<>(); + private Map pvEquivocations = new ConcurrentHashMap<>(); + private Map pcEquivocations = new ConcurrentHashMap<>(); + + public long getThreshold() { + return (THRESHOLD_NUMERATOR * voters.size()) / THRESHOLD_DENOMINATOR; } public BigInteger derivePrimary() { From 8bb9fc7b289f295b469756c85b7f2c33c6610ff0 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Wed, 18 Dec 2024 10:34:43 +0200 Subject: [PATCH 05/18] feat: added all of the auxiliary methods for ghost --- .../com/limechain/grandpa/GrandpaService.java | 50 +++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index a466aa72..fbd1d2bb 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -1,8 +1,10 @@ package com.limechain.grandpa; +import com.limechain.exception.storage.BlockStorageGenericException; import com.limechain.grandpa.state.GrandpaState; import com.limechain.grandpa.state.Subround; import com.limechain.network.protocol.grandpa.messages.commit.Vote; +import com.limechain.network.protocol.warp.dto.BlockHeader; import com.limechain.storage.block.BlockState; import io.emeraldpay.polkaj.types.Hash256; import lombok.extern.java.Log; @@ -28,7 +30,7 @@ public GrandpaService(GrandpaState grandpaState, BlockState blockState) { } //TODO: Implement further - private void getGrandpaGhost(BigInteger roundNumber){ + private void getGrandpaGhost(BigInteger roundNumber) { var threshold = grandpaState.getThreshold(); } @@ -50,13 +52,55 @@ private HashMap getPossibleSelectedBlocks(Long threshold, S List allVotes = getVotes(subround); for (Vote vote : votes.keySet()) { - //TODO: Implement further -// blocks = getPossibleSelectedAncestors(allVotes, v.getHash(), blocks, stage, threshold); + blocks = getPossibleSelectedAncestors(allVotes, vote.getBlockHash(), blocks, subround, threshold); } return blocks; } + public HashMap getPossibleSelectedAncestors(List votes, + Hash256 curr, + HashMap selected, + Subround subround, + long threshold) { + + for (Vote vote : votes) { + if (vote.getBlockHash().equals(curr)) { + continue; + } + + Hash256 pred; + try { + pred = blockState.lowestCommonAncestor(vote.getBlockHash(), curr); + } catch (IllegalArgumentException | BlockStorageGenericException e) { + //TODO: Refactor + continue; + } + + if (pred.equals(curr)) { + return selected; + } + + long totalVotes = getTotalVotesForBlock(pred, subround); + + if (totalVotes > threshold) { + + BlockHeader header = blockState.getHeader(pred); + if (header == null) { + //TODO: Refactor + throw new IllegalStateException("Header not found for block: " + pred); + } + + selected.put(pred, header.getBlockNumber()); + } else { + selected = getPossibleSelectedAncestors(votes, pred, selected, subround, threshold); + } + } + + return selected; + } + + private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { long votesForBlock = getVotesForBlock(blockHash, subround); From 3ec9ec27ec4e08c807380cb6d104a763b61fb3c8 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Wed, 18 Dec 2024 17:50:24 +0200 Subject: [PATCH 06/18] feat: add getGHOST method and comments to all other methods --- .../com/limechain/grandpa/GrandpaService.java | 107 ++++++++++++++---- 1 file changed, 84 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index fbd1d2bb..dea4ee1d 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -29,13 +29,60 @@ public GrandpaService(GrandpaState grandpaState, BlockState blockState) { this.blockState = blockState; } - //TODO: Implement further - private void getGrandpaGhost(BigInteger roundNumber) { + /** + * Finds and returns the block with the most votes in the GRANDPA prevote stage. + * If there are multiple blocks with the same number of votes, selects the block with the highest number. + * If no block meets the criteria, throws an exception indicating no valid GHOST candidate. + */ + public Vote getGrandpaGHOST() { var threshold = grandpaState.getThreshold(); + Map blocks; + // Reduce threshold iteratively until valid blocks are found or threshold reaches zero + while (true) { + blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_VOTE); + if (!blocks.isEmpty() || threshold == 0) { + break; + } + threshold--; + } + + if (blocks.isEmpty()) throw new IllegalStateException("GHOST not found."); + + return selectBlockWithMostVotes(blocks); + } + + /** + * Selects the block with the most votes from the provided map of blocks. + * If multiple blocks have the same number of votes, it returns the one with the highest block number. + * Starts with the last finalized block as the initial candidate. + */ + private Vote selectBlockWithMostVotes(Map blocks) { + var lastFinalizedBlockHeader = blockState.getHighestFinalizedHeader(); + + Vote highest = new Vote( + lastFinalizedBlockHeader.getHash(), + lastFinalizedBlockHeader.getBlockNumber() + ); + + for (Map.Entry entry : blocks.entrySet()) { + Hash256 hash = entry.getKey(); + BigInteger number = entry.getValue(); + + if (number.compareTo(highest.getBlockNumber()) > 0) { + highest = new Vote(hash, number); + } + } + + return highest; } - private HashMap getPossibleSelectedBlocks(Long threshold, Subround subround) { + /** + * Returns blocks with total votes over the threshold in a map of block hash to block number. + * If no blocks meet the threshold directly, recursively searches their ancestors for blocks with enough votes. + * Ancestors are included if their combined votes (including votes for their descendants) exceed the threshold. + */ + private Map getPossibleSelectedBlocks(Long threshold, Subround subround) { var votes = getDirectVotes(subround); var blocks = new HashMap(); @@ -52,17 +99,23 @@ private HashMap getPossibleSelectedBlocks(Long threshold, S List allVotes = getVotes(subround); for (Vote vote : votes.keySet()) { - blocks = getPossibleSelectedAncestors(allVotes, vote.getBlockHash(), blocks, subround, threshold); + blocks = new HashMap<>( + getPossibleSelectedAncestors(allVotes, vote.getBlockHash(), blocks, subround, threshold) + ); } return blocks; } - public HashMap getPossibleSelectedAncestors(List votes, - Hash256 curr, - HashMap selected, - Subround subround, - long threshold) { + /** + * Returns a map of block hash to block number for ancestors meeting the threshold condition. + * Recursively searches for ancestors with more than 2/3 votes. + */ + public Map getPossibleSelectedAncestors(List votes, + Hash256 curr, + Map selected, + Subround subround, + long threshold) { for (Vote vote : votes) { if (vote.getBlockHash().equals(curr)) { @@ -101,8 +154,12 @@ public HashMap getPossibleSelectedAncestors(List vote } + /** + * Calculates the total votes for a block, including observed votes and equivocations, + * in the specified subround. + */ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { - long votesForBlock = getVotesForBlock(blockHash, subround); + long votesForBlock = getObservedVotesForBlock(blockHash, subround); if (votesForBlock == 0L) { return 0L; @@ -118,21 +175,23 @@ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { return votesForBlock + equivocationCount; } - private long getVotesForBlock(Hash256 blockHash, Subround subround) { + /** + * Calculates the total observed votes for a block, including direct votes and votes from + * its descendants, in the specified subround. + */ + private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { var votes = getDirectVotes(subround); - long votesForBlock = 0L; + var votesForBlock = 0L; for (Map.Entry entry : votes.entrySet()) { - Vote v = entry.getKey(); - long count = entry.getValue(); + var vote = entry.getKey(); + var count = entry.getValue(); try { - // Check if the current block is a descendant of the given block - boolean isDescendant = blockState.isDescendantOf(blockHash, v.getBlockHash()); + var isDescendant = blockState.isDescendantOf(blockHash, vote.getBlockHash()); if (isDescendant) { votesForBlock += count; } - } catch (Exception e) { //TODO: adjust the exception type return 0L; @@ -142,19 +201,21 @@ private long getVotesForBlock(Hash256 blockHash, Subround subround) { return votesForBlock; } - // returns a map of votes to direct vote count - private ConcurrentHashMap getDirectVotes(Subround subround) { - var votes = new ConcurrentHashMap(); - var voteCountMap = new ConcurrentHashMap(); + /** + * Aggregates direct (explicit) votes for a given subround into a map of Vote to their count + */ + private HashMap getDirectVotes(Subround subround) { + var votes = new HashMap(); + var voteCounts = new HashMap(); switch (subround) { case Subround.PRE_VOTE -> votes.putAll(grandpaState.getPrevotes()); case Subround.PRE_COMMIT -> votes.putAll(grandpaState.getPrecommits()); } - votes.values().forEach(vote -> voteCountMap.merge(vote, 1L, Long::sum)); + votes.values().forEach(vote -> voteCounts.merge(vote, 1L, Long::sum)); - return voteCountMap; + return voteCounts; } private List getVotes(Subround subround) { From fc5d7f50407996a65af99c2210c73e485b0ce83a Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Thu, 19 Dec 2024 16:51:27 +0200 Subject: [PATCH 07/18] feat: adjust threshold comparison logic --- .../com/limechain/grandpa/GrandpaService.java | 23 ++++++++----------- .../limechain/grandpa/state/GrandpaState.java | 21 +++++++++++++---- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index dea4ee1d..611519cc 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -16,7 +16,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; @Log @Component @@ -38,16 +37,11 @@ public Vote getGrandpaGHOST() { var threshold = grandpaState.getThreshold(); Map blocks; - // Reduce threshold iteratively until valid blocks are found or threshold reaches zero - while (true) { - blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_VOTE); - if (!blocks.isEmpty() || threshold == 0) { - break; - } - threshold--; - } + blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_COMMIT); - if (blocks.isEmpty()) throw new IllegalStateException("GHOST not found."); + if (blocks.isEmpty() || threshold.equals(BigInteger.ZERO)) { + throw new IllegalStateException("GHOST not found."); + } return selectBlockWithMostVotes(blocks); } @@ -82,13 +76,14 @@ private Vote selectBlockWithMostVotes(Map blocks) { * If no blocks meet the threshold directly, recursively searches their ancestors for blocks with enough votes. * Ancestors are included if their combined votes (including votes for their descendants) exceed the threshold. */ - private Map getPossibleSelectedBlocks(Long threshold, Subround subround) { + private Map getPossibleSelectedBlocks(BigInteger threshold, Subround subround) { var votes = getDirectVotes(subround); var blocks = new HashMap(); for (Vote vote : votes.keySet()) { long totalVotes = getTotalVotesForBlock(vote.getBlockHash(), subround); - if (totalVotes > threshold) { + + if (BigInteger.valueOf(totalVotes).compareTo(threshold) >= 0) { blocks.put(vote.getBlockHash(), vote.getBlockNumber()); } } @@ -115,7 +110,7 @@ public Map getPossibleSelectedAncestors(List votes, Hash256 curr, Map selected, Subround subround, - long threshold) { + BigInteger threshold) { for (Vote vote : votes) { if (vote.getBlockHash().equals(curr)) { @@ -136,7 +131,7 @@ public Map getPossibleSelectedAncestors(List votes, long totalVotes = getTotalVotesForBlock(pred, subround); - if (totalVotes > threshold) { + if (BigInteger.valueOf(totalVotes).compareTo(threshold) >= 0) { BlockHeader header = blockState.getHeader(pred); if (header == null) { diff --git a/src/main/java/com/limechain/grandpa/state/GrandpaState.java b/src/main/java/com/limechain/grandpa/state/GrandpaState.java index 8401ce8f..f3376dd1 100644 --- a/src/main/java/com/limechain/grandpa/state/GrandpaState.java +++ b/src/main/java/com/limechain/grandpa/state/GrandpaState.java @@ -21,8 +21,7 @@ @Component public class GrandpaState { - private static final long THRESHOLD_NUMERATOR = 2L; - private static final long THRESHOLD_DENOMINATOR = 3L; + private static final BigInteger THRESHOLD_DENOMINATOR = BigInteger.valueOf(3); private List voters; private BigInteger setId; @@ -34,8 +33,22 @@ public class GrandpaState { private Map pvEquivocations = new ConcurrentHashMap<>(); private Map pcEquivocations = new ConcurrentHashMap<>(); - public long getThreshold() { - return (THRESHOLD_NUMERATOR * voters.size()) / THRESHOLD_DENOMINATOR; + /** + * The threshold is determined as the total weight of authorities + * divided by the weight of potentially faulty authorities (one-third of the total weight minus one). + * + * @return threshold for achieving a super-majority vote + */ + public BigInteger getThreshold() { + var totalWeight = getAuthoritiesTotalWeight(); + var faulty = (totalWeight.subtract(BigInteger.ONE)).divide(THRESHOLD_DENOMINATOR); + return totalWeight.divide(faulty); + } + + public BigInteger getAuthoritiesTotalWeight() { + return voters.stream() + .map(Authority::getWeight) + .reduce(BigInteger.ZERO, BigInteger::add); } public BigInteger derivePrimary() { From 07a9092a2b755d118dc854231dcb6cc3379b6883 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Fri, 20 Dec 2024 14:27:35 +0200 Subject: [PATCH 08/18] chore: refactor --- .../com/limechain/grandpa/GrandpaService.java | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index 611519cc..de82995c 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -16,6 +16,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.logging.Level; @Log @Component @@ -32,6 +33,8 @@ public GrandpaService(GrandpaState grandpaState, BlockState blockState) { * Finds and returns the block with the most votes in the GRANDPA prevote stage. * If there are multiple blocks with the same number of votes, selects the block with the highest number. * If no block meets the criteria, throws an exception indicating no valid GHOST candidate. + * + * @return GRANDPA GHOST block as a vote */ public Vote getGrandpaGHOST() { var threshold = grandpaState.getThreshold(); @@ -40,7 +43,8 @@ public Vote getGrandpaGHOST() { blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_COMMIT); if (blocks.isEmpty() || threshold.equals(BigInteger.ZERO)) { - throw new IllegalStateException("GHOST not found."); + log.log(Level.WARNING, "GHOST not found"); + return null; } return selectBlockWithMostVotes(blocks); @@ -50,6 +54,9 @@ public Vote getGrandpaGHOST() { * Selects the block with the most votes from the provided map of blocks. * If multiple blocks have the same number of votes, it returns the one with the highest block number. * Starts with the last finalized block as the initial candidate. + * + * @param blocks map of block that exceed the required threshold + * @return the block with the most votes from the provided map */ private Vote selectBlockWithMostVotes(Map blocks) { var lastFinalizedBlockHeader = blockState.getHighestFinalizedHeader(); @@ -75,6 +82,10 @@ private Vote selectBlockWithMostVotes(Map blocks) { * Returns blocks with total votes over the threshold in a map of block hash to block number. * If no blocks meet the threshold directly, recursively searches their ancestors for blocks with enough votes. * Ancestors are included if their combined votes (including votes for their descendants) exceed the threshold. + * + * @param threshold minimum votes required for a block to qualify. + * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @return blocks that exceed the required vote threshold */ private Map getPossibleSelectedBlocks(BigInteger threshold, Subround subround) { var votes = getDirectVotes(subround); @@ -103,29 +114,35 @@ private Map getPossibleSelectedBlocks(BigInteger threshold, } /** - * Returns a map of block hash to block number for ancestors meeting the threshold condition. * Recursively searches for ancestors with more than 2/3 votes. + * + * @param votes voters list + * @param currentBlockHash the hash of the current block + * @param selected currently selected block hashes that exceed the required vote threshold + * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param threshold minimum votes required for a block to qualify. + * @return map of block hash to block number for ancestors meeting the threshold condition. */ public Map getPossibleSelectedAncestors(List votes, - Hash256 curr, + Hash256 currentBlockHash, Map selected, Subround subround, BigInteger threshold) { for (Vote vote : votes) { - if (vote.getBlockHash().equals(curr)) { + if (vote.getBlockHash().equals(currentBlockHash)) { continue; } Hash256 pred; try { - pred = blockState.lowestCommonAncestor(vote.getBlockHash(), curr); + pred = blockState.lowestCommonAncestor(vote.getBlockHash(), currentBlockHash); } catch (IllegalArgumentException | BlockStorageGenericException e) { //TODO: Refactor continue; } - if (pred.equals(curr)) { + if (pred.equals(currentBlockHash)) { return selected; } @@ -148,10 +165,13 @@ public Map getPossibleSelectedAncestors(List votes, return selected; } - /** * Calculates the total votes for a block, including observed votes and equivocations, * in the specified subround. + * + * @param blockHash hash of the block + * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @retyrn total votes for a specific block */ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { long votesForBlock = getObservedVotesForBlock(blockHash, subround); @@ -173,6 +193,10 @@ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { /** * Calculates the total observed votes for a block, including direct votes and votes from * its descendants, in the specified subround. + * + * @param blockHash hash of the block + * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @return total observed votes */ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { var votes = getDirectVotes(subround); @@ -198,6 +222,9 @@ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { /** * Aggregates direct (explicit) votes for a given subround into a map of Vote to their count + * + * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @return map of direct votes */ private HashMap getDirectVotes(Subround subround) { var votes = new HashMap(); From 7392728428bcc488cd053cf10c5e474f6fefcfc8 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Fri, 20 Dec 2024 14:33:26 +0200 Subject: [PATCH 09/18] chore: refactor --- .../java/com/limechain/grandpa/GrandpaService.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index de82995c..e0171576 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -116,11 +116,11 @@ private Map getPossibleSelectedBlocks(BigInteger threshold, /** * Recursively searches for ancestors with more than 2/3 votes. * - * @param votes voters list + * @param votes voters list * @param currentBlockHash the hash of the current block - * @param selected currently selected block hashes that exceed the required vote threshold - * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. - * @param threshold minimum votes required for a block to qualify. + * @param selected currently selected block hashes that exceed the required vote threshold + * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param threshold minimum votes required for a block to qualify. * @return map of block hash to block number for ancestors meeting the threshold condition. */ public Map getPossibleSelectedAncestors(List votes, @@ -223,7 +223,7 @@ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { /** * Aggregates direct (explicit) votes for a given subround into a map of Vote to their count * - * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. * @return map of direct votes */ private HashMap getDirectVotes(Subround subround) { From 3ea937140e047d37877573aa6e20d61566a317d9 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Fri, 20 Dec 2024 15:11:05 +0200 Subject: [PATCH 10/18] chore: refactor --- .../com/limechain/grandpa/GrandpaService.java | 57 +++++++++---------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index e0171576..854d9511 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -16,7 +16,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.logging.Level; @Log @Component @@ -38,12 +37,11 @@ public GrandpaService(GrandpaState grandpaState, BlockState blockState) { */ public Vote getGrandpaGHOST() { var threshold = grandpaState.getThreshold(); - Map blocks; - blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_COMMIT); + Map blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_COMMIT); if (blocks.isEmpty() || threshold.equals(BigInteger.ZERO)) { - log.log(Level.WARNING, "GHOST not found"); + log.warning("GHOST not found"); return null; } @@ -134,31 +132,32 @@ public Map getPossibleSelectedAncestors(List votes, continue; } - Hash256 pred; + Hash256 ancestorBlockHash; try { - pred = blockState.lowestCommonAncestor(vote.getBlockHash(), currentBlockHash); + ancestorBlockHash = blockState.lowestCommonAncestor(vote.getBlockHash(), currentBlockHash); } catch (IllegalArgumentException | BlockStorageGenericException e) { - //TODO: Refactor + log.warning("Error finding the lowest common ancestor: " + e.getMessage()); continue; } - if (pred.equals(currentBlockHash)) { + // Happens when currentBlock is ancestor of the block in the vote + if (ancestorBlockHash.equals(currentBlockHash)) { return selected; } - long totalVotes = getTotalVotesForBlock(pred, subround); + long totalVotes = getTotalVotesForBlock(ancestorBlockHash, subround); if (BigInteger.valueOf(totalVotes).compareTo(threshold) >= 0) { - BlockHeader header = blockState.getHeader(pred); + BlockHeader header = blockState.getHeader(ancestorBlockHash); if (header == null) { - //TODO: Refactor - throw new IllegalStateException("Header not found for block: " + pred); + throw new IllegalStateException("Header not found for block: " + ancestorBlockHash); } - selected.put(pred, header.getBlockNumber()); + selected.put(ancestorBlockHash, header.getBlockNumber()); } else { - selected = getPossibleSelectedAncestors(votes, pred, selected, subround, threshold); + // Recursively process ancestors + selected = getPossibleSelectedAncestors(votes, ancestorBlockHash, selected, subround, threshold); } } @@ -180,12 +179,11 @@ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { return 0L; } - int equivocationCount = 0; - - switch (subround) { - case Subround.PRE_VOTE -> equivocationCount = grandpaState.getPvEquivocations().size(); - case Subround.PRE_COMMIT -> equivocationCount = grandpaState.getPcEquivocations().size(); - } + int equivocationCount = switch (subround) { + case Subround.PRE_VOTE -> grandpaState.getPvEquivocations().size(); + case Subround.PRE_COMMIT -> grandpaState.getPcEquivocations().size(); + default -> 0; + }; return votesForBlock + equivocationCount; } @@ -207,13 +205,14 @@ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { var count = entry.getValue(); try { - var isDescendant = blockState.isDescendantOf(blockHash, vote.getBlockHash()); - if (isDescendant) { + if (blockState.isDescendantOf(blockHash, vote.getBlockHash())) { votesForBlock += count; } + } catch (BlockStorageGenericException e) { + log.warning(e.getMessage()); + return 0L; // Default to zero votes in case of block state error } catch (Exception e) { - //TODO: adjust the exception type - return 0L; + log.warning("An error occurred while checking block ancestry: " + e.getMessage()); } } @@ -227,13 +226,13 @@ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { * @return map of direct votes */ private HashMap getDirectVotes(Subround subround) { - var votes = new HashMap(); var voteCounts = new HashMap(); - switch (subround) { - case Subround.PRE_VOTE -> votes.putAll(grandpaState.getPrevotes()); - case Subround.PRE_COMMIT -> votes.putAll(grandpaState.getPrecommits()); - } + Map votes = switch (subround) { + case Subround.PRE_VOTE -> grandpaState.getPrevotes(); + case Subround.PRE_COMMIT -> grandpaState.getPrecommits(); + default -> new HashMap<>(); + }; votes.values().forEach(vote -> voteCounts.merge(vote, 1L, Long::sum)); From 28cd621a33f4636bfc65772e8c3c33030abac2c7 Mon Sep 17 00:00:00 2001 From: Georgi Grigorov Date: Fri, 20 Dec 2024 17:30:15 +0200 Subject: [PATCH 11/18] feat: add unit tests --- .../com/limechain/grandpa/GrandpaService.java | 8 +- .../limechain/grandpa/state/GrandpaState.java | 10 +- .../limechain/grandpa/GrandpaServiceTest.java | 192 ++++++++++++++++++ 3 files changed, 202 insertions(+), 8 deletions(-) create mode 100644 src/test/java/com/limechain/grandpa/GrandpaServiceTest.java diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index 854d9511..4cf6911a 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -7,8 +7,8 @@ import com.limechain.network.protocol.warp.dto.BlockHeader; import com.limechain.storage.block.BlockState; import io.emeraldpay.polkaj.types.Hash256; +import io.libp2p.core.crypto.PubKey; import lombok.extern.java.Log; -import org.bouncycastle.math.ec.rfc8032.Ed25519; import org.springframework.stereotype.Component; import java.math.BigInteger; @@ -38,7 +38,7 @@ public GrandpaService(GrandpaState grandpaState, BlockState blockState) { public Vote getGrandpaGHOST() { var threshold = grandpaState.getThreshold(); - Map blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_COMMIT); + Map blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_VOTE); if (blocks.isEmpty() || threshold.equals(BigInteger.ZERO)) { log.warning("GHOST not found"); @@ -121,7 +121,7 @@ private Map getPossibleSelectedBlocks(BigInteger threshold, * @param threshold minimum votes required for a block to qualify. * @return map of block hash to block number for ancestors meeting the threshold condition. */ - public Map getPossibleSelectedAncestors(List votes, + private Map getPossibleSelectedAncestors(List votes, Hash256 currentBlockHash, Map selected, Subround subround, @@ -228,7 +228,7 @@ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { private HashMap getDirectVotes(Subround subround) { var voteCounts = new HashMap(); - Map votes = switch (subround) { + Map votes = switch (subround) { case Subround.PRE_VOTE -> grandpaState.getPrevotes(); case Subround.PRE_COMMIT -> grandpaState.getPrecommits(); default -> new HashMap<>(); diff --git a/src/main/java/com/limechain/grandpa/state/GrandpaState.java b/src/main/java/com/limechain/grandpa/state/GrandpaState.java index f3376dd1..4c4694d0 100644 --- a/src/main/java/com/limechain/grandpa/state/GrandpaState.java +++ b/src/main/java/com/limechain/grandpa/state/GrandpaState.java @@ -3,6 +3,8 @@ import com.limechain.chain.lightsyncstate.Authority; import com.limechain.network.protocol.grandpa.messages.catchup.res.SignedVote; import com.limechain.network.protocol.grandpa.messages.commit.Vote; +import io.libp2p.core.crypto.PubKey; +import io.libp2p.crypto.keys.Ed25519PublicKey; import lombok.Getter; import org.bouncycastle.math.ec.rfc8032.Ed25519; import org.springframework.stereotype.Component; @@ -28,10 +30,10 @@ public class GrandpaState { private BigInteger roundNumber; //TODO: This may not be the best place for those maps - private Map precommits = new ConcurrentHashMap<>(); - private Map prevotes = new ConcurrentHashMap<>(); - private Map pvEquivocations = new ConcurrentHashMap<>(); - private Map pcEquivocations = new ConcurrentHashMap<>(); + private Map precommits = new ConcurrentHashMap<>(); + private Map prevotes = new ConcurrentHashMap<>(); + private Map pvEquivocations = new ConcurrentHashMap<>(); + private Map pcEquivocations = new ConcurrentHashMap<>(); /** * The threshold is determined as the total weight of authorities diff --git a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java new file mode 100644 index 00000000..d3f0d5d8 --- /dev/null +++ b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java @@ -0,0 +1,192 @@ +package com.limechain.grandpa; + +import com.limechain.grandpa.state.GrandpaState; +import com.limechain.grandpa.state.Subround; +import com.limechain.network.protocol.grandpa.messages.commit.Vote; +import com.limechain.network.protocol.warp.dto.BlockHeader; +import com.limechain.network.protocol.warp.dto.ConsensusEngine; +import com.limechain.network.protocol.warp.dto.DigestType; +import com.limechain.network.protocol.warp.dto.HeaderDigest; +import com.limechain.storage.block.BlockState; +import com.limechain.utils.Ed25519Utils; +import io.emeraldpay.polkaj.types.Hash256; +import io.libp2p.core.crypto.PubKey; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Method; +import java.math.BigInteger; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static com.limechain.utils.TestUtils.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class GrandpaServiceTest { + + private static final byte[] ZEROS_ARRAY = new byte[32]; + private static final byte[] ONES_ARRAY = + new byte[]{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}; + private static final byte[] TWOS_ARRAY = + new byte[]{2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2}; + + private GrandpaState grandpaState; + private BlockState blockState; + private GrandpaService grandpaService; + + @BeforeEach + void setUp() { + grandpaState = mock(GrandpaState.class); + blockState = mock(BlockState.class); + grandpaService = new GrandpaService(grandpaState, blockState); + } + + @Test + void testGetGrandpaGHOSTWhereNoBlocksPassThreshold() { + when(grandpaState.getThreshold()).thenReturn(BigInteger.valueOf(10)); + when(grandpaState.getPrevotes()).thenReturn(Map.of()); + + var result = grandpaService.getGrandpaGHOST(); + assertNull(result); + } + + @Test + void testGetGrandpaGHOSTWithBlockPassingThreshold() { + when(grandpaState.getThreshold()).thenReturn(BigInteger.valueOf(1)); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + + when(grandpaState.getPrevotes()).thenReturn(Map.of( + Ed25519Utils.generateKeyPair().publicKey(), firstVote, + Ed25519Utils.generateKeyPair().publicKey(), secondVote + )); + + BlockHeader blockHeader = createBlockHeader(); + blockHeader.setBlockNumber(BigInteger.valueOf(1)); + when(blockState.getHighestFinalizedHeader()).thenReturn(blockHeader); + + when(blockState.isDescendantOf(firstVote.getBlockHash(), firstVote.getBlockHash())).thenReturn(true); + when(blockState.isDescendantOf(secondVote.getBlockHash(), secondVote.getBlockHash())).thenReturn(true); + + Vote result = grandpaService.getGrandpaGHOST(); + assertNotNull(result); + assertEquals(firstVote.getBlockHash(), result.getBlockHash()); + } + + @Test + void testGetDirectVotesForPrevotes() throws Exception { + // Prepare mock data + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + + // Call the private method via reflection + Method method = GrandpaService.class.getDeclaredMethod("getDirectVotes", Subround.class); + method.setAccessible(true); + + Map result = (HashMap) method.invoke(grandpaService, Subround.PRE_VOTE); + + assertEquals(1L, result.get(firstVote)); + assertEquals(1L, result.get(secondVote)); + } + + @Test + void testGetDirectVotesWithMultipleVotesForSingleBlockForPrevotes() throws Exception { + // Prepare mock data + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + + // Call the private method via reflection + Method method = GrandpaService.class.getDeclaredMethod("getDirectVotes", Subround.class); + method.setAccessible(true); + + Map result = (HashMap) method.invoke(grandpaService, Subround.PRE_VOTE); + + assertEquals(2L, result.get(firstVote)); + } + + @Test + void testGetVotes() throws Exception { + // Prepare mock data + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + + // Call the private method via reflection + Method method = GrandpaService.class.getDeclaredMethod("getVotes", Subround.class); + method.setAccessible(true); + + List result = (List) method.invoke(grandpaService, Subround.PRE_VOTE); + + assertTrue(result.contains(firstVote)); + } + + @Test + void testGetVotesWithMultipleVotes() throws Exception { + // Prepare mock data + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + + // Call the private method via reflection + Method method = GrandpaService.class.getDeclaredMethod("getVotes", Subround.class); + method.setAccessible(true); + + List result = (List) method.invoke(grandpaService, Subround.PRE_VOTE); + + assertTrue(result.contains(firstVote)); + assertTrue(result.contains(secondVote)); + } + + private BlockHeader createBlockHeader() { + HeaderDigest headerDigest = new HeaderDigest(); + headerDigest.setType(DigestType.CONSENSUS_MESSAGE); + headerDigest.setId(ConsensusEngine.GRANDPA); + headerDigest.setMessage(ZEROS_ARRAY); + + BlockHeader blockHeader = new BlockHeader(); + blockHeader.setBlockNumber(BigInteger.valueOf(1)); + blockHeader.setParentHash(new Hash256(ZEROS_ARRAY)); + blockHeader.setDigest(new HeaderDigest[]{headerDigest}); + blockHeader.setStateRoot(new Hash256(ZEROS_ARRAY)); + blockHeader.setExtrinsicsRoot(new Hash256(ZEROS_ARRAY)); + + return blockHeader; + } +} From e0bedbe79ddc50ad5df9112de32cc5018e2ffbc2 Mon Sep 17 00:00:00 2001 From: Grigorov-Georgi Date: Thu, 2 Jan 2025 10:06:48 +0200 Subject: [PATCH 12/18] feat: add grandpaState tests and rework grandpaState class --- .../limechain/grandpa/state/GrandpaState.java | 10 +-- .../grandpa/state/GrandpaStateTest.java | 69 +++++++++++++++++++ 2 files changed, 74 insertions(+), 5 deletions(-) create mode 100644 src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java diff --git a/src/main/java/com/limechain/grandpa/state/GrandpaState.java b/src/main/java/com/limechain/grandpa/state/GrandpaState.java index 4c4694d0..a05f286a 100644 --- a/src/main/java/com/limechain/grandpa/state/GrandpaState.java +++ b/src/main/java/com/limechain/grandpa/state/GrandpaState.java @@ -4,9 +4,8 @@ import com.limechain.network.protocol.grandpa.messages.catchup.res.SignedVote; import com.limechain.network.protocol.grandpa.messages.commit.Vote; import io.libp2p.core.crypto.PubKey; -import io.libp2p.crypto.keys.Ed25519PublicKey; import lombok.Getter; -import org.bouncycastle.math.ec.rfc8032.Ed25519; +import lombok.Setter; import org.springframework.stereotype.Component; import java.math.BigInteger; @@ -20,6 +19,7 @@ * Note: Intended for use only when the host is configured as an Authoring Node. */ @Getter +@Setter @Component public class GrandpaState { @@ -37,17 +37,17 @@ public class GrandpaState { /** * The threshold is determined as the total weight of authorities - * divided by the weight of potentially faulty authorities (one-third of the total weight minus one). + * subtracted by the weight of potentially faulty authorities (one-third of the total weight minus one). * * @return threshold for achieving a super-majority vote */ public BigInteger getThreshold() { var totalWeight = getAuthoritiesTotalWeight(); var faulty = (totalWeight.subtract(BigInteger.ONE)).divide(THRESHOLD_DENOMINATOR); - return totalWeight.divide(faulty); + return totalWeight.subtract(faulty); } - public BigInteger getAuthoritiesTotalWeight() { + private BigInteger getAuthoritiesTotalWeight() { return voters.stream() .map(Authority::getWeight) .reduce(BigInteger.ZERO, BigInteger::add); diff --git a/src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java b/src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java new file mode 100644 index 00000000..436e430a --- /dev/null +++ b/src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java @@ -0,0 +1,69 @@ +package com.limechain.grandpa.state; + +import com.limechain.chain.lightsyncstate.Authority; +import com.limechain.utils.Ed25519Utils; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.math.BigInteger; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +@ExtendWith(MockitoExtension.class) +class GrandpaStateTest { + + @InjectMocks + private GrandpaState grandpaState; + + @Test + void testGetThreshold() { + Authority authority1 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority2 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority3 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority4 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority5 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority6 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority7 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority8 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority9 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority10 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + + grandpaState.setVoters( + List.of( + authority1, authority2, authority3, authority4, authority5, + authority6, authority7, authority8, authority9, authority10 + ) + ); + + // Total weight: 10 + // Faulty: (10 - 1) / 3 = 3 + // Threshold: 10 - faulty = 7 + assertEquals(BigInteger.valueOf(7), grandpaState.getThreshold()); + } + + @Test + void testDerivePrimary() { + Authority authority1 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority2 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + Authority authority3 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); + + grandpaState.setVoters(List.of( + authority1, authority2, authority3 + )); + + // 4 % voters.size = 1 + grandpaState.setRoundNumber(BigInteger.valueOf(4)); + assertEquals(BigInteger.ONE, grandpaState.derivePrimary()); + + // 5 % voters.size = 2 + grandpaState.setRoundNumber(BigInteger.valueOf(5)); + assertEquals(BigInteger.TWO, grandpaState.derivePrimary()); + + // 6 % voters.size = 0 + grandpaState.setRoundNumber(BigInteger.valueOf(6)); + assertEquals(BigInteger.ZERO, grandpaState.derivePrimary()); + } +} From 4fd4822fcea8710e1add7d3eafacf6e8a7fa0673 Mon Sep 17 00:00:00 2001 From: Grigorov-Georgi Date: Thu, 2 Jan 2025 15:23:00 +0200 Subject: [PATCH 13/18] feat: add more tests to grandpa service methods --- .../limechain/grandpa/GrandpaServiceTest.java | 270 ++++++++++++++++++ 1 file changed, 270 insertions(+) diff --git a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java index d3f0d5d8..3ed79212 100644 --- a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java +++ b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java @@ -2,6 +2,7 @@ import com.limechain.grandpa.state.GrandpaState; import com.limechain.grandpa.state.Subround; +import com.limechain.network.protocol.grandpa.messages.catchup.res.SignedVote; import com.limechain.network.protocol.grandpa.messages.commit.Vote; import com.limechain.network.protocol.warp.dto.BlockHeader; import com.limechain.network.protocol.warp.dto.ConsensusEngine; @@ -24,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -34,6 +36,8 @@ class GrandpaServiceTest { new byte[]{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}; private static final byte[] TWOS_ARRAY = new byte[]{2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2}; + private static final byte[] THREES_ARRAY = + new byte[]{3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3}; private GrandpaState grandpaState; private BlockState blockState; @@ -174,6 +178,272 @@ void testGetVotesWithMultipleVotes() throws Exception { assertTrue(result.contains(secondVote)); } + @Test + void testGetObservedVotesForBlockWhereVotesAreNotDescendantsOfProvidedBlockHash() throws Exception { + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + when(blockState.isDescendantOf(any(), any())).thenReturn(false); + + Method method = GrandpaService.class.getDeclaredMethod( + "getObservedVotesForBlock", + Hash256.class, + Subround.class + ); + + method.setAccessible(true); + + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + + assertEquals(0L, result); + } + + @Test + void testGetObservedVotesForBlockWhereVotesAreDescendantsOfProvidedBlockHash() throws Exception { + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + when(blockState.isDescendantOf(any(), any())).thenReturn(true); + + Method method = GrandpaService.class.getDeclaredMethod( + "getObservedVotesForBlock", + Hash256.class, + Subround.class + ); + + method.setAccessible(true); + + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + + assertEquals(2L, result); + } + + @Test + void testGetTotalVotesForBlockWithoutObservedVotes() throws Exception { + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + PubKey pubKey3 = Ed25519Utils.generateKeyPair().publicKey(); + + Map pvEquivocations = new HashMap<>(); + pvEquivocations.put(pubKey3, new SignedVote()); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + when(grandpaState.getPvEquivocations()).thenReturn(pvEquivocations); + when(blockState.isDescendantOf(any(), any())).thenReturn(false); + + Method method = GrandpaService.class.getDeclaredMethod( + "getTotalVotesForBlock", Hash256.class, Subround.class); + + method.setAccessible(true); + + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + + // Observed votes: 0 + // Equivocations: 1 + // Total votes: 0 + assertEquals(0, result); + } + + @Test + void testGetTotalVotesForBlockWithObservedVotes() throws Exception { + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + when(grandpaState.getPvEquivocations()).thenReturn(new HashMap<>()); + when(blockState.isDescendantOf(any(), any())).thenReturn(true); + + Method method = GrandpaService.class.getDeclaredMethod( + "getTotalVotesForBlock", Hash256.class, Subround.class); + + method.setAccessible(true); + + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + + // Observed votes: 2 + // Equivocations: 0 + // Total votes: 2 + assertEquals(2, result); + } + + @Test + void testGetTotalVotesForBlockWithObservedVotesAndEquivocations() throws Exception { + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + PubKey pubKey2 = Ed25519Utils.generateKeyPair().publicKey(); + + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + prevotes.put(pubKey2, secondVote); + + PubKey pubKey3 = Ed25519Utils.generateKeyPair().publicKey(); + + Map pvEquivocations = new HashMap<>(); + pvEquivocations.put(pubKey3, new SignedVote()); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + when(grandpaState.getPvEquivocations()).thenReturn(pvEquivocations); + when(blockState.isDescendantOf(any(), any())).thenReturn(true); + + Method method = GrandpaService.class.getDeclaredMethod( + "getTotalVotesForBlock", Hash256.class, Subround.class); + + method.setAccessible(true); + + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + + // Observed votes: 2 + // Equivocations: 1 + // Total votes: 3 + assertEquals(3, result); + } + + @Test + void testGetPossibleSelectedAncestors() throws Exception { + // ZEROS_ARRAY Block is parent of ONES- and THREES_ARRAY Blocks + // + // ZEROS_ARRAY_BLOCK --> ONES_ARRAY_BLOCK (block from votes) + // | + // --> THREES_ARRAY_BLOCK (current block) + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + List votes = List.of(firstVote); + + PubKey pubKey1 = Ed25519Utils.generateKeyPair().publicKey(); + + Map prevotes = new HashMap<>(); + prevotes.put(pubKey1, firstVote); + + when(grandpaState.getPrevotes()).thenReturn(prevotes); + when(grandpaState.getPvEquivocations()).thenReturn(new HashMap<>()); + when(blockState.isDescendantOf(any(), any())).thenReturn(true); + + when(blockState.lowestCommonAncestor(new Hash256(ONES_ARRAY), new Hash256(THREES_ARRAY))) + .thenReturn(new Hash256(ZEROS_ARRAY)); + + when(blockState.getHeader(new Hash256(ZEROS_ARRAY))) + .thenReturn(createBlockHeader()); + + Method method = GrandpaService.class.getDeclaredMethod("getPossibleSelectedAncestors", + List.class, + Hash256.class, + Map.class, + Subround.class, + BigInteger.class + ); + + method.setAccessible(true); + + Map selected = new HashMap<>(); + + Map result = (Map) method.invoke( + grandpaService, + votes, + new Hash256(THREES_ARRAY), + selected, + Subround.PRE_VOTE, + BigInteger.valueOf(1) + ); + + assertEquals(1, result.size()); + assertTrue(result.containsKey(new Hash256(ZEROS_ARRAY))); + } + + + @Test + void testGetPossibleSelectedBlocksThatAreOverThreshold() throws Exception { + Vote firstVote = new Vote(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + Vote secondVote = new Vote(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + when(grandpaState.getPrevotes()).thenReturn(Map.of( + Ed25519Utils.generateKeyPair().publicKey(), firstVote, + Ed25519Utils.generateKeyPair().publicKey(), secondVote + )); + + when(blockState.isDescendantOf(any(), any())).thenReturn(true); + when(grandpaState.getThreshold()).thenReturn(BigInteger.valueOf(3)); + + Method method = GrandpaService.class.getDeclaredMethod( + "getPossibleSelectedBlocks", BigInteger.class, Subround.class); + method.setAccessible(true); + + Map result = (Map) method.invoke( + grandpaService, BigInteger.valueOf(1), Subround.PRE_VOTE); + + assertEquals(2, result.size()); + assertTrue(result.containsKey(new Hash256(ONES_ARRAY))); + assertTrue(result.containsKey(new Hash256(TWOS_ARRAY))); + } + + @Test + void testSelectBlockWithMostVotes() throws Exception { + Map blocks = new HashMap<>(); + blocks.put(new Hash256(ONES_ARRAY), BigInteger.valueOf(3)); + blocks.put(new Hash256(TWOS_ARRAY), BigInteger.valueOf(4)); + + BlockHeader blockHeader = createBlockHeader(); + when(blockState.getHighestFinalizedHeader()).thenReturn(blockHeader); + + Method method = GrandpaService.class.getDeclaredMethod("selectBlockWithMostVotes", Map.class); + method.setAccessible(true); + + Vote result = (Vote) method.invoke(grandpaService, blocks); + + assertNotNull(result); + assertEquals(new Hash256(TWOS_ARRAY), result.getBlockHash()); + assertEquals(BigInteger.valueOf(4), result.getBlockNumber()); + } + + @Test + void testSelectBlockWithMostVotesWhereLastFinalizedBlockIsWithGreaterBlockNumber() throws Exception { + Map blocks = new HashMap<>(); + blocks.put(new Hash256(ONES_ARRAY), BigInteger.valueOf(0)); + + BlockHeader blockHeader = createBlockHeader(); + when(blockState.getHighestFinalizedHeader()).thenReturn(blockHeader); + + Method method = GrandpaService.class.getDeclaredMethod("selectBlockWithMostVotes", Map.class); + method.setAccessible(true); + + Vote result = (Vote) method.invoke(grandpaService, blocks); + + assertNotNull(result); + assertEquals(blockHeader.getHash(), result.getBlockHash()); + assertEquals(blockHeader.getBlockNumber(), result.getBlockNumber()); + } + private BlockHeader createBlockHeader() { HeaderDigest headerDigest = new HeaderDigest(); headerDigest.setType(DigestType.CONSENSUS_MESSAGE); From 2fa850f4a474e39ac1caf28df70410eaa6f1db6a Mon Sep 17 00:00:00 2001 From: Grigorov-Georgi Date: Thu, 2 Jan 2025 15:31:54 +0200 Subject: [PATCH 14/18] chore: refactor --- src/main/java/com/limechain/grandpa/GrandpaService.java | 4 ++-- src/test/java/com/limechain/grandpa/GrandpaServiceTest.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index 4cf6911a..72bde1ae 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -1,5 +1,6 @@ package com.limechain.grandpa; +import com.limechain.exception.global.ExecutionFailedException; import com.limechain.exception.storage.BlockStorageGenericException; import com.limechain.grandpa.state.GrandpaState; import com.limechain.grandpa.state.Subround; @@ -41,8 +42,7 @@ public Vote getGrandpaGHOST() { Map blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_VOTE); if (blocks.isEmpty() || threshold.equals(BigInteger.ZERO)) { - log.warning("GHOST not found"); - return null; + throw new ExecutionFailedException("GHOST not found"); } return selectBlockWithMostVotes(blocks); diff --git a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java index 3ed79212..755acbdf 100644 --- a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java +++ b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java @@ -1,5 +1,6 @@ package com.limechain.grandpa; +import com.limechain.exception.global.ExecutionFailedException; import com.limechain.grandpa.state.GrandpaState; import com.limechain.grandpa.state.Subround; import com.limechain.network.protocol.grandpa.messages.catchup.res.SignedVote; @@ -24,6 +25,7 @@ import static com.limechain.utils.TestUtils.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -54,9 +56,7 @@ void setUp() { void testGetGrandpaGHOSTWhereNoBlocksPassThreshold() { when(grandpaState.getThreshold()).thenReturn(BigInteger.valueOf(10)); when(grandpaState.getPrevotes()).thenReturn(Map.of()); - - var result = grandpaService.getGrandpaGHOST(); - assertNull(result); + assertThrows(ExecutionFailedException.class, () -> grandpaService.getGrandpaGHOST()); } @Test From d69aa86e61108e493f33b136257fa46e6684bd90 Mon Sep 17 00:00:00 2001 From: Grigorov-Georgi Date: Fri, 3 Jan 2025 10:09:18 +0200 Subject: [PATCH 15/18] chore: remove derivePrimary method and the test for it --- .../limechain/grandpa/state/GrandpaState.java | 7 +----- .../grandpa/state/GrandpaStateTest.java | 23 ------------------- 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/src/main/java/com/limechain/grandpa/state/GrandpaState.java b/src/main/java/com/limechain/grandpa/state/GrandpaState.java index a05f286a..ba46d3eb 100644 --- a/src/main/java/com/limechain/grandpa/state/GrandpaState.java +++ b/src/main/java/com/limechain/grandpa/state/GrandpaState.java @@ -19,7 +19,7 @@ * Note: Intended for use only when the host is configured as an Authoring Node. */ @Getter -@Setter +@Setter //TODO: remove it when initialize() method is implemented @Component public class GrandpaState { @@ -52,9 +52,4 @@ private BigInteger getAuthoritiesTotalWeight() { .map(Authority::getWeight) .reduce(BigInteger.ZERO, BigInteger::add); } - - public BigInteger derivePrimary() { - var votersCount = BigInteger.valueOf(voters.size()); - return roundNumber.remainder(votersCount); - } } diff --git a/src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java b/src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java index 436e430a..4db9d468 100644 --- a/src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java +++ b/src/test/java/com/limechain/grandpa/state/GrandpaStateTest.java @@ -43,27 +43,4 @@ void testGetThreshold() { // Threshold: 10 - faulty = 7 assertEquals(BigInteger.valueOf(7), grandpaState.getThreshold()); } - - @Test - void testDerivePrimary() { - Authority authority1 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); - Authority authority2 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); - Authority authority3 = new Authority(Ed25519Utils.generateKeyPair().publicKey().bytes(), BigInteger.ONE); - - grandpaState.setVoters(List.of( - authority1, authority2, authority3 - )); - - // 4 % voters.size = 1 - grandpaState.setRoundNumber(BigInteger.valueOf(4)); - assertEquals(BigInteger.ONE, grandpaState.derivePrimary()); - - // 5 % voters.size = 2 - grandpaState.setRoundNumber(BigInteger.valueOf(5)); - assertEquals(BigInteger.TWO, grandpaState.derivePrimary()); - - // 6 % voters.size = 0 - grandpaState.setRoundNumber(BigInteger.valueOf(6)); - assertEquals(BigInteger.ZERO, grandpaState.derivePrimary()); - } } From 373a724b593a90471bc780b8c756698a3b3d163c Mon Sep 17 00:00:00 2001 From: Grigorov-Georgi Date: Fri, 3 Jan 2025 13:04:18 +0200 Subject: [PATCH 16/18] feat: remove duplicating subround enum --- .../com/limechain/grandpa/GrandpaService.java | 22 ++++++++-------- .../com/limechain/grandpa/state/Subround.java | 9 ------- .../limechain/grandpa/GrandpaServiceTest.java | 25 +++++++++---------- 3 files changed, 23 insertions(+), 33 deletions(-) delete mode 100644 src/main/java/com/limechain/grandpa/state/Subround.java diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index 72bde1ae..55b33d80 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -3,8 +3,8 @@ import com.limechain.exception.global.ExecutionFailedException; import com.limechain.exception.storage.BlockStorageGenericException; import com.limechain.grandpa.state.GrandpaState; -import com.limechain.grandpa.state.Subround; import com.limechain.network.protocol.grandpa.messages.commit.Vote; +import com.limechain.network.protocol.grandpa.messages.vote.Subround; import com.limechain.network.protocol.warp.dto.BlockHeader; import com.limechain.storage.block.BlockState; import io.emeraldpay.polkaj.types.Hash256; @@ -39,7 +39,7 @@ public GrandpaService(GrandpaState grandpaState, BlockState blockState) { public Vote getGrandpaGHOST() { var threshold = grandpaState.getThreshold(); - Map blocks = getPossibleSelectedBlocks(threshold, Subround.PRE_VOTE); + Map blocks = getPossibleSelectedBlocks(threshold, Subround.PREVOTE); if (blocks.isEmpty() || threshold.equals(BigInteger.ZERO)) { throw new ExecutionFailedException("GHOST not found"); @@ -82,7 +82,7 @@ private Vote selectBlockWithMostVotes(Map blocks) { * Ancestors are included if their combined votes (including votes for their descendants) exceed the threshold. * * @param threshold minimum votes required for a block to qualify. - * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param subround stage of the GRANDPA process, such as PREVOTE, PRECOMMIT or PRIMARY_PROPOSAL. * @return blocks that exceed the required vote threshold */ private Map getPossibleSelectedBlocks(BigInteger threshold, Subround subround) { @@ -117,7 +117,7 @@ private Map getPossibleSelectedBlocks(BigInteger threshold, * @param votes voters list * @param currentBlockHash the hash of the current block * @param selected currently selected block hashes that exceed the required vote threshold - * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param subround stage of the GRANDPA process, such as PREVOTE, PRECOMMIT or PRIMARY_PROPOSAL. * @param threshold minimum votes required for a block to qualify. * @return map of block hash to block number for ancestors meeting the threshold condition. */ @@ -169,7 +169,7 @@ private Map getPossibleSelectedAncestors(List votes, * in the specified subround. * * @param blockHash hash of the block - * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param subround stage of the GRANDPA process, such as PREVOTE, PRECOMMIT or PRIMARY_PROPOSAL. * @retyrn total votes for a specific block */ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { @@ -180,8 +180,8 @@ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { } int equivocationCount = switch (subround) { - case Subround.PRE_VOTE -> grandpaState.getPvEquivocations().size(); - case Subround.PRE_COMMIT -> grandpaState.getPcEquivocations().size(); + case Subround.PREVOTE -> grandpaState.getPvEquivocations().size(); + case Subround.PRECOMMIT -> grandpaState.getPcEquivocations().size(); default -> 0; }; @@ -193,7 +193,7 @@ private long getTotalVotesForBlock(Hash256 blockHash, Subround subround) { * its descendants, in the specified subround. * * @param blockHash hash of the block - * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param subround stage of the GRANDPA process, such as PREVOTE, PRECOMMIT or PRIMARY_PROPOSAL. * @return total observed votes */ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { @@ -222,15 +222,15 @@ private long getObservedVotesForBlock(Hash256 blockHash, Subround subround) { /** * Aggregates direct (explicit) votes for a given subround into a map of Vote to their count * - * @param subround stage of the GRANDPA process, such as PRE_VOTE, PRE_COMMIT or PRIMARY_PROPOSAL. + * @param subround stage of the GRANDPA process, such as PREVOTE, PRECOMMIT or PRIMARY_PROPOSAL. * @return map of direct votes */ private HashMap getDirectVotes(Subround subround) { var voteCounts = new HashMap(); Map votes = switch (subround) { - case Subround.PRE_VOTE -> grandpaState.getPrevotes(); - case Subround.PRE_COMMIT -> grandpaState.getPrecommits(); + case Subround.PREVOTE -> grandpaState.getPrevotes(); + case Subround.PRECOMMIT -> grandpaState.getPrecommits(); default -> new HashMap<>(); }; diff --git a/src/main/java/com/limechain/grandpa/state/Subround.java b/src/main/java/com/limechain/grandpa/state/Subround.java deleted file mode 100644 index 58e2a46e..00000000 --- a/src/main/java/com/limechain/grandpa/state/Subround.java +++ /dev/null @@ -1,9 +0,0 @@ -package com.limechain.grandpa.state; - -public enum Subround { - PRE_VOTE(0), - PRE_COMMIT(1), - PRIMARY_PROPOSAL(2); - - Subround(int value) {} -} diff --git a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java index 755acbdf..1686c4f1 100644 --- a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java +++ b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java @@ -2,9 +2,9 @@ import com.limechain.exception.global.ExecutionFailedException; import com.limechain.grandpa.state.GrandpaState; -import com.limechain.grandpa.state.Subround; import com.limechain.network.protocol.grandpa.messages.catchup.res.SignedVote; import com.limechain.network.protocol.grandpa.messages.commit.Vote; +import com.limechain.network.protocol.grandpa.messages.vote.Subround; import com.limechain.network.protocol.warp.dto.BlockHeader; import com.limechain.network.protocol.warp.dto.ConsensusEngine; import com.limechain.network.protocol.warp.dto.DigestType; @@ -24,7 +24,6 @@ import static com.limechain.utils.TestUtils.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -102,7 +101,7 @@ void testGetDirectVotesForPrevotes() throws Exception { Method method = GrandpaService.class.getDeclaredMethod("getDirectVotes", Subround.class); method.setAccessible(true); - Map result = (HashMap) method.invoke(grandpaService, Subround.PRE_VOTE); + Map result = (HashMap) method.invoke(grandpaService, Subround.PREVOTE); assertEquals(1L, result.get(firstVote)); assertEquals(1L, result.get(secondVote)); @@ -127,7 +126,7 @@ void testGetDirectVotesWithMultipleVotesForSingleBlockForPrevotes() throws Excep Method method = GrandpaService.class.getDeclaredMethod("getDirectVotes", Subround.class); method.setAccessible(true); - Map result = (HashMap) method.invoke(grandpaService, Subround.PRE_VOTE); + Map result = (HashMap) method.invoke(grandpaService, Subround.PREVOTE); assertEquals(2L, result.get(firstVote)); } @@ -148,7 +147,7 @@ void testGetVotes() throws Exception { Method method = GrandpaService.class.getDeclaredMethod("getVotes", Subround.class); method.setAccessible(true); - List result = (List) method.invoke(grandpaService, Subround.PRE_VOTE); + List result = (List) method.invoke(grandpaService, Subround.PREVOTE); assertTrue(result.contains(firstVote)); } @@ -172,7 +171,7 @@ void testGetVotesWithMultipleVotes() throws Exception { Method method = GrandpaService.class.getDeclaredMethod("getVotes", Subround.class); method.setAccessible(true); - List result = (List) method.invoke(grandpaService, Subround.PRE_VOTE); + List result = (List) method.invoke(grandpaService, Subround.PREVOTE); assertTrue(result.contains(firstVote)); assertTrue(result.contains(secondVote)); @@ -201,7 +200,7 @@ void testGetObservedVotesForBlockWhereVotesAreNotDescendantsOfProvidedBlockHash( method.setAccessible(true); - long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PREVOTE); assertEquals(0L, result); } @@ -229,7 +228,7 @@ void testGetObservedVotesForBlockWhereVotesAreDescendantsOfProvidedBlockHash() t method.setAccessible(true); - long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PREVOTE); assertEquals(2L, result); } @@ -260,7 +259,7 @@ void testGetTotalVotesForBlockWithoutObservedVotes() throws Exception { method.setAccessible(true); - long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PREVOTE); // Observed votes: 0 // Equivocations: 1 @@ -289,7 +288,7 @@ void testGetTotalVotesForBlockWithObservedVotes() throws Exception { method.setAccessible(true); - long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PREVOTE); // Observed votes: 2 // Equivocations: 0 @@ -323,7 +322,7 @@ void testGetTotalVotesForBlockWithObservedVotesAndEquivocations() throws Excepti method.setAccessible(true); - long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PRE_VOTE); + long result = (long) method.invoke(grandpaService, new Hash256(ZEROS_ARRAY), Subround.PREVOTE); // Observed votes: 2 // Equivocations: 1 @@ -373,7 +372,7 @@ void testGetPossibleSelectedAncestors() throws Exception { votes, new Hash256(THREES_ARRAY), selected, - Subround.PRE_VOTE, + Subround.PREVOTE, BigInteger.valueOf(1) ); @@ -400,7 +399,7 @@ void testGetPossibleSelectedBlocksThatAreOverThreshold() throws Exception { method.setAccessible(true); Map result = (Map) method.invoke( - grandpaService, BigInteger.valueOf(1), Subround.PRE_VOTE); + grandpaService, BigInteger.valueOf(1), Subround.PREVOTE); assertEquals(2, result.size()); assertTrue(result.containsKey(new Hash256(ONES_ARRAY))); From 9a8e387217c6f006826d06c36bae18ae6998fec3 Mon Sep 17 00:00:00 2001 From: Grigorov-Georgi Date: Fri, 3 Jan 2025 13:23:49 +0200 Subject: [PATCH 17/18] chore: refactor --- .../exception/grandpa/GhostExecutionException.java | 7 +++++++ .../exception/grandpa/GrandpaGenericException.java | 7 +++++++ src/main/java/com/limechain/grandpa/GrandpaService.java | 7 ++++--- .../java/com/limechain/grandpa/GrandpaServiceTest.java | 4 ++-- 4 files changed, 20 insertions(+), 5 deletions(-) create mode 100644 src/main/java/com/limechain/exception/grandpa/GhostExecutionException.java create mode 100644 src/main/java/com/limechain/exception/grandpa/GrandpaGenericException.java diff --git a/src/main/java/com/limechain/exception/grandpa/GhostExecutionException.java b/src/main/java/com/limechain/exception/grandpa/GhostExecutionException.java new file mode 100644 index 00000000..087ac158 --- /dev/null +++ b/src/main/java/com/limechain/exception/grandpa/GhostExecutionException.java @@ -0,0 +1,7 @@ +package com.limechain.exception.grandpa; + +public class GhostExecutionException extends GrandpaGenericException { + public GhostExecutionException(String message) { + super(message); + } +} diff --git a/src/main/java/com/limechain/exception/grandpa/GrandpaGenericException.java b/src/main/java/com/limechain/exception/grandpa/GrandpaGenericException.java new file mode 100644 index 00000000..7d4f33f2 --- /dev/null +++ b/src/main/java/com/limechain/exception/grandpa/GrandpaGenericException.java @@ -0,0 +1,7 @@ +package com.limechain.exception.grandpa; + +public class GrandpaGenericException extends RuntimeException { + public GrandpaGenericException(String message) { + super(message); + } +} diff --git a/src/main/java/com/limechain/grandpa/GrandpaService.java b/src/main/java/com/limechain/grandpa/GrandpaService.java index 55b33d80..e923bb25 100644 --- a/src/main/java/com/limechain/grandpa/GrandpaService.java +++ b/src/main/java/com/limechain/grandpa/GrandpaService.java @@ -1,6 +1,6 @@ package com.limechain.grandpa; -import com.limechain.exception.global.ExecutionFailedException; +import com.limechain.exception.grandpa.GhostExecutionException; import com.limechain.exception.storage.BlockStorageGenericException; import com.limechain.grandpa.state.GrandpaState; import com.limechain.network.protocol.grandpa.messages.commit.Vote; @@ -21,6 +21,7 @@ @Log @Component public class GrandpaService { + private final GrandpaState grandpaState; private final BlockState blockState; @@ -36,13 +37,13 @@ public GrandpaService(GrandpaState grandpaState, BlockState blockState) { * * @return GRANDPA GHOST block as a vote */ - public Vote getGrandpaGHOST() { + public Vote getGrandpaGhost() { var threshold = grandpaState.getThreshold(); Map blocks = getPossibleSelectedBlocks(threshold, Subround.PREVOTE); if (blocks.isEmpty() || threshold.equals(BigInteger.ZERO)) { - throw new ExecutionFailedException("GHOST not found"); + throw new GhostExecutionException("GHOST not found"); } return selectBlockWithMostVotes(blocks); diff --git a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java index 1686c4f1..11716c81 100644 --- a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java +++ b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java @@ -55,7 +55,7 @@ void setUp() { void testGetGrandpaGHOSTWhereNoBlocksPassThreshold() { when(grandpaState.getThreshold()).thenReturn(BigInteger.valueOf(10)); when(grandpaState.getPrevotes()).thenReturn(Map.of()); - assertThrows(ExecutionFailedException.class, () -> grandpaService.getGrandpaGHOST()); + assertThrows(ExecutionFailedException.class, () -> grandpaService.getGrandpaGhost()); } @Test @@ -77,7 +77,7 @@ void testGetGrandpaGHOSTWithBlockPassingThreshold() { when(blockState.isDescendantOf(firstVote.getBlockHash(), firstVote.getBlockHash())).thenReturn(true); when(blockState.isDescendantOf(secondVote.getBlockHash(), secondVote.getBlockHash())).thenReturn(true); - Vote result = grandpaService.getGrandpaGHOST(); + Vote result = grandpaService.getGrandpaGhost(); assertNotNull(result); assertEquals(firstVote.getBlockHash(), result.getBlockHash()); } From e7fab2d005d3ac5680368fb537e19599b219e9d3 Mon Sep 17 00:00:00 2001 From: Grigorov-Georgi Date: Fri, 3 Jan 2025 13:59:35 +0200 Subject: [PATCH 18/18] chore: fix failing test --- src/test/java/com/limechain/grandpa/GrandpaServiceTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java index 11716c81..e99cc163 100644 --- a/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java +++ b/src/test/java/com/limechain/grandpa/GrandpaServiceTest.java @@ -1,6 +1,6 @@ package com.limechain.grandpa; -import com.limechain.exception.global.ExecutionFailedException; +import com.limechain.exception.grandpa.GhostExecutionException; import com.limechain.grandpa.state.GrandpaState; import com.limechain.network.protocol.grandpa.messages.catchup.res.SignedVote; import com.limechain.network.protocol.grandpa.messages.commit.Vote; @@ -55,7 +55,7 @@ void setUp() { void testGetGrandpaGHOSTWhereNoBlocksPassThreshold() { when(grandpaState.getThreshold()).thenReturn(BigInteger.valueOf(10)); when(grandpaState.getPrevotes()).thenReturn(Map.of()); - assertThrows(ExecutionFailedException.class, () -> grandpaService.getGrandpaGhost()); + assertThrows(GhostExecutionException.class, () -> grandpaService.getGrandpaGhost()); } @Test