Qbft vote encoding to use empty for drop for extradata to match quorum (#2232)

Signed-off-by: Jason Frame <jasonwframe@gmail.com>

consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/Vote.java

@@ -16,14 +16,9 @@ package org.hyperledger.besu.consensus.common.bft;
 
 import org.hyperledger.besu.consensus.common.VoteType;
 import org.hyperledger.besu.ethereum.core.Address;
-import org.hyperledger.besu.ethereum.rlp.RLPException;
-import org.hyperledger.besu.ethereum.rlp.RLPInput;
-import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 
 import java.util.Objects;
 
-import com.google.common.collect.ImmutableBiMap;
-
 /**
  * This class is only used to serialise/deserialise BlockHeaders and should not appear in business
  * logic.
@@ -35,11 +30,6 @@ public class Vote {
   public static final byte ADD_BYTE_VALUE = (byte) 0xFF;
   public static final byte DROP_BYTE_VALUE = (byte) 0x0L;
 
-  private static final ImmutableBiMap<VoteType, Byte> voteToValue =
-      ImmutableBiMap.of(
-          VoteType.ADD, ADD_BYTE_VALUE,
-          VoteType.DROP, DROP_BYTE_VALUE);
-
   public Vote(final Address recipient, final VoteType voteType) {
     this.recipient = recipient;
     this.voteType = voteType;
@@ -81,24 +71,4 @@ public class Vote {
   public int hashCode() {
     return Objects.hash(recipient, voteType);
   }
-
-  public void writeTo(final RLPOutput rlpOutput) {
-    rlpOutput.startList();
-    rlpOutput.writeBytes(recipient);
-    rlpOutput.writeByte(voteToValue.get(voteType));
-    rlpOutput.endList();
-  }
-
-  public static Vote readFrom(final RLPInput rlpInput) {
-
-    rlpInput.enterList();
-    final Address recipient = Address.readFrom(rlpInput);
-    final VoteType vote = voteToValue.inverse().get(rlpInput.readByte());
-    if (vote == null) {
-      throw new RLPException("Vote field was of an incorrect binary value.");
-    }
-    rlpInput.leaveList();
-
-    return new Vote(recipient, vote);
-  }
 }

consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/IbftExtraDataCodec.java

@@ -14,6 +14,10 @@
  */
 package org.hyperledger.besu.consensus.ibft;
 
+import static org.hyperledger.besu.consensus.common.bft.Vote.ADD_BYTE_VALUE;
+import static org.hyperledger.besu.consensus.common.bft.Vote.DROP_BYTE_VALUE;
+
+import org.hyperledger.besu.consensus.common.VoteType;
 import org.hyperledger.besu.consensus.common.bft.BftExtraData;
 import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.Vote;
@@ -22,13 +26,16 @@ import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
+import org.hyperledger.besu.ethereum.rlp.RLPException;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
+import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
+import com.google.common.collect.ImmutableBiMap;
 import org.apache.tuweni.bytes.Bytes;
 
 /**
@@ -36,6 +43,10 @@ import org.apache.tuweni.bytes.Bytes;
  * operating under an BFT consensus mechanism.
  */
 public class IbftExtraDataCodec extends BftExtraDataCodec {
+  private static final ImmutableBiMap<VoteType, Byte> voteToValue =
+      ImmutableBiMap.of(
+          VoteType.ADD, ADD_BYTE_VALUE,
+          VoteType.DROP, DROP_BYTE_VALUE);
 
   public static Bytes encodeFromAddresses(final Collection<Address> addresses) {
     return new IbftExtraDataCodec()
@@ -68,7 +79,7 @@ public class IbftExtraDataCodec extends BftExtraDataCodec {
       vote = Optional.empty();
       rlpInput.skipNext();
     } else {
-      vote = Optional.of(Vote.readFrom(rlpInput));
+      vote = Optional.of(decodeVote(rlpInput));
     }
     final int round = rlpInput.readInt();
     final List<SECPSignature> seals =
@@ -86,7 +97,7 @@ public class IbftExtraDataCodec extends BftExtraDataCodec {
     encoder.writeBytes(bftExtraData.getVanityData());
     encoder.writeList(bftExtraData.getValidators(), (validator, rlp) -> rlp.writeBytes(validator));
     if (bftExtraData.getVote().isPresent()) {
-      bftExtraData.getVote().get().writeTo(encoder);
+      encodeVote(encoder, bftExtraData.getVote().get());
     } else {
       encoder.writeNull();
     }
@@ -102,4 +113,24 @@ public class IbftExtraDataCodec extends BftExtraDataCodec {
 
     return encoder.encoded();
   }
+
+  private void encodeVote(final RLPOutput rlpOutput, final Vote vote) {
+    final VoteType voteType = vote.isAuth() ? VoteType.ADD : VoteType.DROP;
+    rlpOutput.startList();
+    rlpOutput.writeBytes(vote.getRecipient());
+    rlpOutput.writeByte(voteToValue.get(voteType));
+    rlpOutput.endList();
+  }
+
+  private Vote decodeVote(final RLPInput rlpInput) {
+    rlpInput.enterList();
+    final Address recipient = Address.readFrom(rlpInput);
+    final VoteType vote = voteToValue.inverse().get(rlpInput.readByte());
+    if (vote == null) {
+      throw new RLPException("Vote field was of an incorrect binary value.");
+    }
+    rlpInput.leaveList();
+
+    return new Vote(recipient, vote);
+  }
 }

consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/IbftExtraDataEncoderTest.java

@@ -34,8 +34,8 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import java.util.Random;
+import java.util.function.Supplier;
 
-import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.Lists;
 import org.apache.tuweni.bytes.Bytes;
@@ -77,7 +77,6 @@ public class IbftExtraDataEncoderTest {
   @Test
   public void correctlyCodedRoundConstitutesValidContent() {
     final List<Address> validators = Lists.newArrayList();
-    final Optional<Vote> vote = Optional.of(Vote.authVote(Address.fromHexString("1")));
     final int round = 0x00FEDCBA;
     final byte[] roundAsByteArray = new byte[] {(byte) 0x00, (byte) 0xFE, (byte) 0xDC, (byte) 0xBA};
     final List<SECPSignature> committerSeals = Lists.newArrayList();
@@ -92,7 +91,10 @@ public class IbftExtraDataEncoderTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(Address.fromHexString("1"));
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     // This is to verify that the decoding works correctly when the round is encoded as 4 bytes
     encoder.writeBytes(Bytes.wrap(roundAsByteArray));
@@ -116,7 +118,6 @@ public class IbftExtraDataEncoderTest {
   @Test
   public void incorrectlyEncodedRoundThrowsRlpException() {
     final List<Address> validators = Lists.newArrayList();
-    final Optional<Vote> vote = Optional.of(Vote.authVote(Address.fromHexString("1")));
     final byte[] roundAsByteArray = new byte[] {(byte) 0xFE, (byte) 0xDC, (byte) 0xBA};
     final List<SECPSignature> committerSeals = Lists.newArrayList();
 
@@ -130,7 +131,10 @@ public class IbftExtraDataEncoderTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(Address.fromHexString("1"));
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     // This is to verify that the decoding throws an exception when the round number is not encoded
     // in 4 byte format
@@ -201,7 +205,6 @@ public class IbftExtraDataEncoderTest {
   @Test
   public void emptyListConstituteValidContent() {
     final List<Address> validators = Lists.newArrayList();
-    final Optional<Vote> vote = Optional.of(Vote.dropVote(Address.fromHexString("1")));
     final int round = 0x00FEDCBA;
     final List<SECPSignature> committerSeals = Lists.newArrayList();
 
@@ -215,7 +218,10 @@ public class IbftExtraDataEncoderTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(Address.fromHexString("1"));
+    encoder.writeByte(Vote.DROP_BYTE_VALUE);
+    encoder.endList();
 
     encoder.writeInt(round);
     encoder.writeList(committerSeals, (committer, rlp) -> rlp.writeBytes(committer.encodedBytes()));
@@ -358,7 +364,10 @@ public class IbftExtraDataEncoderTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(vote.get().getRecipient());
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     encoder.writeInt(round);
     encoder.endList();
@@ -393,7 +402,10 @@ public class IbftExtraDataEncoderTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(vote.get().getRecipient());
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     encoder.endList();
 
@@ -409,7 +421,6 @@ public class IbftExtraDataEncoderTest {
   @Test
   public void incorrectlyStructuredRlpThrowsException() {
     final List<Address> validators = Lists.newArrayList();
-    final Optional<Vote> vote = Optional.of(Vote.authVote(Address.fromHexString("1")));
     final int round = 0x00FEDCBA;
     final List<SECPSignature> committerSeals = Lists.newArrayList();
 
@@ -423,7 +434,10 @@ public class IbftExtraDataEncoderTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(Address.fromHexString("1"));
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     encoder.writeInt(round);
     encoder.writeList(committerSeals, (committer, rlp) -> rlp.writeBytes(committer.encodedBytes()));

consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/QbftExtraDataCodec.java

@@ -14,6 +14,10 @@
  */
 package org.hyperledger.besu.consensus.qbft;
 
+import static org.hyperledger.besu.consensus.common.bft.Vote.ADD_BYTE_VALUE;
+import static org.hyperledger.besu.consensus.common.bft.Vote.DROP_BYTE_VALUE;
+
+import org.hyperledger.besu.consensus.common.VoteType;
 import org.hyperledger.besu.consensus.common.bft.BftExtraData;
 import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.Vote;
@@ -22,13 +26,16 @@ import org.hyperledger.besu.crypto.SignatureAlgorithmFactory;
 import org.hyperledger.besu.ethereum.core.Address;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPInput;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
+import org.hyperledger.besu.ethereum.rlp.RLPException;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
+import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
+import com.google.common.collect.ImmutableBiMap;
 import org.apache.tuweni.bytes.Bytes;
 
 /**
@@ -36,6 +43,10 @@ import org.apache.tuweni.bytes.Bytes;
  * operating under an BFT consensus mechanism.
  */
 public class QbftExtraDataCodec extends BftExtraDataCodec {
+  private static final ImmutableBiMap<VoteType, Byte> voteToValue =
+      ImmutableBiMap.of(
+          VoteType.ADD, ADD_BYTE_VALUE,
+          VoteType.DROP, DROP_BYTE_VALUE);
 
   public static Bytes encodeFromAddresses(final Collection<Address> addresses) {
     return new QbftExtraDataCodec()
@@ -69,7 +80,7 @@ public class QbftExtraDataCodec extends BftExtraDataCodec {
       vote = Optional.empty();
       rlpInput.skipNext();
     } else {
-      vote = Optional.of(Vote.readFrom(rlpInput));
+      vote = Optional.of(decodeVote(rlpInput));
     }
 
     final int round = rlpInput.readIntScalar();
@@ -89,7 +100,7 @@ public class QbftExtraDataCodec extends BftExtraDataCodec {
     encoder.writeList(bftExtraData.getValidators(), (validator, rlp) -> rlp.writeBytes(validator));
 
     if (bftExtraData.getVote().isPresent()) {
-      bftExtraData.getVote().get().writeTo(encoder);
+      encodeVote(encoder, bftExtraData.getVote().get());
     } else {
       encoder.writeList(Collections.emptyList(), (o, rlpOutput) -> {});
     }
@@ -110,4 +121,36 @@ public class QbftExtraDataCodec extends BftExtraDataCodec {
 
     return encoder.encoded();
   }
+
+  private void encodeVote(final RLPOutput rlpOutput, final Vote vote) {
+    final VoteType voteType = vote.isAuth() ? VoteType.ADD : VoteType.DROP;
+    rlpOutput.startList();
+    rlpOutput.writeBytes(vote.getRecipient());
+    if (voteType == VoteType.ADD) {
+      rlpOutput.writeByte(ADD_BYTE_VALUE);
+    } else {
+      rlpOutput.writeNull();
+    }
+    rlpOutput.endList();
+  }
+
+  private Vote decodeVote(final RLPInput rlpInput) {
+    rlpInput.enterList();
+    final Address recipient = Address.readFrom(rlpInput);
+
+    final VoteType vote;
+    if (rlpInput.nextSize() == 0) {
+      rlpInput.skipNext();
+      vote = VoteType.DROP;
+    } else {
+      vote = voteToValue.inverse().get(rlpInput.readByte());
+    }
+
+    if (vote == null) {
+      throw new RLPException("Vote field was of an incorrect binary value.");
+    }
+    rlpInput.leaveList();
+
+    return new Vote(recipient, vote);
+  }
 }

consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/QbftExtraDataCodecTest.java

@@ -34,8 +34,8 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import java.util.Random;
+import java.util.function.Supplier;
 
-import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.common.collect.Lists;
 import org.apache.tuweni.bytes.Bytes;
@@ -92,7 +92,10 @@ public class QbftExtraDataCodecTest {
     encoder.writeBytes(vanity_data);
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(vote.get().getRecipient());
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     // This is to verify that the decoding works correctly when the round is encoded as non-fixed
     // length bytes
@@ -130,8 +133,8 @@ public class QbftExtraDataCodecTest {
     encoder.writeBytes(vanity_data);
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
-    // encoded vote
+    encoder.writeBytes(vote.get().getRecipient());
-    vote.get().writeTo(encoder);
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
 
     // This is to verify that the decoding throws an exception when the round number is encoded in
     // 4 byte format
@@ -215,7 +218,10 @@ public class QbftExtraDataCodecTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(vote.get().getRecipient());
+    encoder.writeNull();
+    encoder.endList();
 
     encoder.writeIntScalar(round);
     encoder.writeList(committerSeals, (committer, rlp) -> rlp.writeBytes(committer.encodedBytes()));
@@ -355,7 +361,10 @@ public class QbftExtraDataCodecTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(Address.fromHexString("1"));
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     encoder.writeIntScalar(round);
     encoder.writeEmptyList();
@@ -367,6 +376,8 @@ public class QbftExtraDataCodecTest {
         bftExtraDataCodec.encodeWithoutCommitSeals(
             new BftExtraData(vanity_data, committerSeals, vote, round, validators));
 
+    System.out.println("actualEncoding = " + actualEncoding);
+
     assertThat(actualEncoding).isEqualTo(expectedEncoding);
   }
 
@@ -391,7 +402,10 @@ public class QbftExtraDataCodecTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(Address.fromHexString("1"));
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     encoder.writeNull();
     encoder.writeEmptyList();
@@ -410,7 +424,6 @@ public class QbftExtraDataCodecTest {
   @Test
   public void incorrectlyStructuredRlpThrowsException() {
     final List<Address> validators = Lists.newArrayList();
-    final Optional<Vote> vote = Optional.of(Vote.authVote(Address.fromHexString("1")));
     final int round = 0x00FEDCBA;
     final List<SECPSignature> committerSeals = Lists.newArrayList();
 
@@ -424,7 +437,10 @@ public class QbftExtraDataCodecTest {
     encoder.writeList(validators, (validator, rlp) -> rlp.writeBytes(validator));
 
     // encoded vote
-    vote.get().writeTo(encoder);
+    encoder.startList();
+    encoder.writeBytes(Address.fromHexString("1"));
+    encoder.writeByte(Vote.ADD_BYTE_VALUE);
+    encoder.endList();
 
     encoder.writeInt(round);
     encoder.writeList(committerSeals, (committer, rlp) -> rlp.writeBytes(committer.encodedBytes()));