[bisq-network/bisq] Dao fix issues with majority hash (#2360)

[sqrrm]

sqrrm approved this pull request.

utACK

Added some fixes to spelling mistakes and variable naming.

Otherwise this looks better, waiting for the end of batch parsing and checking the current phase at that height before publishing vote. I wonder if our usage of chainHeight, blocHeight, height and what not is lacking since we're getting confused. Perhaps better naming would help us understand better what we're doing.

>      // The chain height of the latest Block we either get reported by Bitcoin Core or from the seed node
+    // This property should not be used in consensus code but only or retrieving blocks as it is not in sync with the

```suggestion
    // This property should not be used in consensus code but only for retrieving blocks as it is not in sync with the
```

>      // the blind vote was created in case we have not done it already.
     // The voter need to be at least once online in the reveal phase when he has a blind vote created,
-    // otherwise his vote becomes invalid and his locked stake will get unlocked
-    private void maybeRevealVotes() {
-        // We must not use daoStateService.getChainHeight() because that gets updated with each parsed block but we
-        // only want to publish the vote reveal tx if our current real chain height is matching the cycle and phase and
-        // not at any intermediate height during parsing all blocks. The bsqNode knows the latest height from either
-        // Bitcoin Core or from the seed node.
-        int chainHeight = bsqNode.getChainTipHeight();
+    // otherwise his vote becomes invalid.
+    // In case the user miss the vote reveal phase an (invalid) vote reveal tx will be created the next time the user is
+    // online. That tx only serves the purpose to unlock the stake from teh blind vote but it will be ignored for voting.

```suggestion
    // online. That tx only serves the purpose to unlock the stake from the blind vote but it will be ignored for voting.
```

>                      } else {
+                        // We missed the vote reveal phase but publish a vote reveal tx to unlock he blind vote stake.

```suggestion
                        // We missed the vote reveal phase but publish a vote reveal tx to unlock the blind vote stake.
```

>          try {
             // We collect all valid blind vote items we received via the p2p network.
             // It might be that different nodes have a different collection of those items.
             // To ensure we get a consensus of the data for later calculating the result we will put a hash of each
-            // voters  blind vote collection into the opReturn data and check for a majority at issuance time.
+            // voters  blind vote collection into the opReturn data and check for a majority at the vote result phase.

```suggestion
            // voter's blind vote collection into the opReturn data and check for a majority in the vote result phase.
```

> @@ -28,17 +28,17 @@
 
 public class VoteResultException extends Exception {
     @Getter
-    private final Cycle cycle;
+    private final int heightOfFirstBlock;

The name `heightOfFirstBlock` is a bit confusing without the context that it's the first block in the cycle

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/2360#pullrequestreview-199392927
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20190203/9cc783af/attachment-0001.html>