Refactor QbftExtraDataCodec from new instance to injection (hyperledg…

…er#2645) * Removed static codec on RoundChange * Removed static codec on ProposalPayload Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
lucassaldanha · Aug 18, 2021 · 6039730 · 6039730
1 parent c651092
commit 6039730
Show file tree

Hide file tree

Showing 17 changed files with 78 additions and 42 deletions.
diff --git a/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/QbftBesuControllerBuilder.java
@@ -166,7 +166,7 @@ protected MiningCoordinator createMiningCoordinator(
     final UniqueMessageMulticaster uniqueMessageMulticaster =
         new UniqueMessageMulticaster(peers, qbftConfig.getGossipedHistoryLimit());
 
-    final QbftGossip gossiper = new QbftGossip(uniqueMessageMulticaster);
+    final QbftGossip gossiper = new QbftGossip(uniqueMessageMulticaster, bftExtraDataCodec().get());
 
     final BftFinalState finalState =
         new BftFinalState(
@@ -217,7 +217,8 @@ protected MiningCoordinator createMiningCoordinator(
             gossiper,
             duplicateMessageTracker,
             futureMessageBuffer,
-            new EthSynchronizerUpdater(ethProtocolManager.ethContext().getEthPeers()));
+            new EthSynchronizerUpdater(ethProtocolManager.ethContext().getEthPeers()),
+            bftExtraDataCodec().get());
 
     final EventMultiplexer eventMultiplexer = new EventMultiplexer(qbftController);
     final BftProcessor bftProcessor = new BftProcessor(bftEventQueue, eventMultiplexer);

diff --git a/...integration-test/java/org/hyperledger/besu/consensus/qbft/support/RoundSpecificPeers.java b/...integration-test/java/org/hyperledger/besu/consensus/qbft/support/RoundSpecificPeers.java
@@ -18,6 +18,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Fail.fail;
 
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier;
 import org.hyperledger.besu.consensus.common.bft.messagewrappers.BftMessage;
 import org.hyperledger.besu.consensus.common.bft.payload.Payload;
@@ -51,14 +52,17 @@ public class RoundSpecificPeers {
   private final ValidatorPeer proposer;
   private final Collection<ValidatorPeer> peers;
   private final List<ValidatorPeer> nonProposingPeers;
+  private final BftExtraDataCodec bftExtraDataCodec;
 
   public RoundSpecificPeers(
       final ValidatorPeer proposer,
       final Collection<ValidatorPeer> peers,
-      final List<ValidatorPeer> nonProposingPeers) {
+      final List<ValidatorPeer> nonProposingPeers,
+      final BftExtraDataCodec bftExtraDataCodec) {
     this.proposer = proposer;
     this.peers = peers;
     this.nonProposingPeers = nonProposingPeers;
+    this.bftExtraDataCodec = bftExtraDataCodec;
   }
 
   public ValidatorPeer getProposer() {
@@ -196,7 +200,7 @@ private void verifyMessage(final MessageData actual, final BftMessage<?> expecte
 
     switch (expectedMessage.getMessageType()) {
       case QbftV1.PROPOSAL:
-        actualSignedPayload = ProposalMessageData.fromMessageData(actual).decode();
+        actualSignedPayload = ProposalMessageData.fromMessageData(actual).decode(bftExtraDataCodec);
         break;
       case QbftV1.PREPARE:
         actualSignedPayload = PrepareMessageData.fromMessageData(actual).decode();
@@ -205,7 +209,8 @@ private void verifyMessage(final MessageData actual, final BftMessage<?> expecte
         actualSignedPayload = CommitMessageData.fromMessageData(actual).decode();
         break;
       case QbftV1.ROUND_CHANGE:
-        actualSignedPayload = RoundChangeMessageData.fromMessageData(actual).decode();
+        actualSignedPayload =
+            RoundChangeMessageData.fromMessageData(actual).decode(bftExtraDataCodec);
         break;
       default:
         fail("Illegal QBFTV1 message type.");

diff --git a/...ft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContext.java b/...ft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContext.java
@@ -127,7 +127,7 @@ public RoundSpecificPeers roundSpecificPeers(final ConsensusRoundIdentifier roun
     final List<ValidatorPeer> nonProposers = new ArrayList<>(remotePeers.values());
     nonProposers.remove(proposer);
 
-    return new RoundSpecificPeers(proposer, remotePeers.values(), nonProposers);
+    return new RoundSpecificPeers(proposer, remotePeers.values(), nonProposers, bftExtraDataCodec);
   }
 
   public NodeParams getLocalNodeParams() {

diff --git a/...integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java b/...integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java
@@ -262,7 +262,10 @@ public TestContext build() {
     final UniqueMessageMulticaster uniqueMulticaster =
         new UniqueMessageMulticaster(multicaster, GOSSIPED_HISTORY_LIMIT);
 
-    final Gossiper gossiper = useGossip ? new QbftGossip(uniqueMulticaster) : mock(Gossiper.class);
+    final Gossiper gossiper =
+        useGossip
+            ? new QbftGossip(uniqueMulticaster, BFT_EXTRA_DATA_ENCODER)
+            : mock(Gossiper.class);
 
     final StubbedSynchronizerUpdater synchronizerUpdater = new StubbedSynchronizerUpdater();
 
@@ -469,7 +472,8 @@ private static ControllerAndState createControllerAndFinalState(
             gossiper,
             duplicateMessageTracker,
             futureMessageBuffer,
-            synchronizerUpdater);
+            synchronizerUpdater,
+            BFT_EXTRA_DATA_ENCODER);
 
     final EventMultiplexer eventMultiplexer = new EventMultiplexer(qbftController);
     //////////////////////////// END IBFT BesuController ////////////////////////////

diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/QbftGossip.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/QbftGossip.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.consensus.qbft;
 
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.Gossiper;
 import org.hyperledger.besu.consensus.common.bft.network.ValidatorMulticaster;
 import org.hyperledger.besu.consensus.common.bft.payload.Authored;
@@ -34,14 +35,18 @@
 public class QbftGossip implements Gossiper {
 
   private final ValidatorMulticaster multicaster;
+  private final BftExtraDataCodec bftExtraDataCodec;
 
   /**
    * Constructor that attaches gossip logic to a set of multicaster
    *
    * @param multicaster Network connections to the remote validators
+   * @param bftExtraDataCodec Codec used when decoding MessageData
    */
-  public QbftGossip(final ValidatorMulticaster multicaster) {
+  public QbftGossip(
+      final ValidatorMulticaster multicaster, final BftExtraDataCodec bftExtraDataCodec) {
     this.multicaster = multicaster;
+    this.bftExtraDataCodec = bftExtraDataCodec;
   }
 
   /**
@@ -55,7 +60,7 @@ public void send(final Message message) {
     final Authored decodedMessage;
     switch (messageData.getCode()) {
       case QbftV1.PROPOSAL:
-        decodedMessage = ProposalMessageData.fromMessageData(messageData).decode();
+        decodedMessage = ProposalMessageData.fromMessageData(messageData).decode(bftExtraDataCodec);
         break;
       case QbftV1.PREPARE:
         decodedMessage = PrepareMessageData.fromMessageData(messageData).decode();
@@ -64,7 +69,8 @@ public void send(final Message message) {
         decodedMessage = CommitMessageData.fromMessageData(messageData).decode();
         break;
       case QbftV1.ROUND_CHANGE:
-        decodedMessage = RoundChangeMessageData.fromMessageData(messageData).decode();
+        decodedMessage =
+            RoundChangeMessageData.fromMessageData(messageData).decode(bftExtraDataCodec);
         break;
       default:
         throw new IllegalArgumentException(

diff --git a/...ft/src/main/java/org/hyperledger/besu/consensus/qbft/messagedata/ProposalMessageData.java b/...ft/src/main/java/org/hyperledger/besu/consensus/qbft/messagedata/ProposalMessageData.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.consensus.qbft.messagedata;
 
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.messagedata.AbstractBftMessageData;
 import org.hyperledger.besu.consensus.qbft.messagewrappers.Proposal;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
@@ -33,8 +34,8 @@ public static ProposalMessageData fromMessageData(final MessageData messageData)
         messageData, MESSAGE_CODE, ProposalMessageData.class, ProposalMessageData::new);
   }
 
-  public Proposal decode() {
-    return Proposal.decode(data);
+  public Proposal decode(final BftExtraDataCodec bftExtraDataCodec) {
+    return Proposal.decode(data, bftExtraDataCodec);
   }
 
   public static ProposalMessageData create(final Proposal proposal) {

diff --git a/...src/main/java/org/hyperledger/besu/consensus/qbft/messagedata/RoundChangeMessageData.java b/...src/main/java/org/hyperledger/besu/consensus/qbft/messagedata/RoundChangeMessageData.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.consensus.qbft.messagedata;
 
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.messagedata.AbstractBftMessageData;
 import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange;
 import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData;
@@ -33,8 +34,8 @@ public static RoundChangeMessageData fromMessageData(final MessageData messageDa
         messageData, MESSAGE_CODE, RoundChangeMessageData.class, RoundChangeMessageData::new);
   }
 
-  public RoundChange decode() {
-    return RoundChange.decode(data);
+  public RoundChange decode(final BftExtraDataCodec bftExtraDataCodec) {
+    return RoundChange.decode(data, bftExtraDataCodec);
   }
 
   public static RoundChangeMessageData create(final RoundChange signedPayload) {

diff --git a/...nsus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/Proposal.java b/...nsus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/Proposal.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.consensus.qbft.messagewrappers;
 
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.messagewrappers.BftMessage;
 import org.hyperledger.besu.consensus.common.bft.payload.SignedData;
 import org.hyperledger.besu.consensus.qbft.payload.PreparePayload;
@@ -69,10 +70,11 @@ public Bytes encode() {
     return rlpOut.encoded();
   }
 
-  public static Proposal decode(final Bytes data) {
+  public static Proposal decode(final Bytes data, final BftExtraDataCodec bftExtraDataCodec) {
     final RLPInput rlpIn = RLP.input(data);
     rlpIn.enterList();
-    final SignedData<ProposalPayload> payload = readPayload(rlpIn, ProposalPayload::readFrom);
+    final SignedData<ProposalPayload> payload =
+        readPayload(rlpIn, rlpInput -> ProposalPayload.readFrom(rlpInput, bftExtraDataCodec));
 
     rlpIn.enterList();
     final List<SignedData<RoundChangePayload>> roundChanges =

diff --git a/...s/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/RoundChange.java b/...s/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/RoundChange.java
@@ -15,9 +15,9 @@
 package org.hyperledger.besu.consensus.qbft.messagewrappers;
 
 import org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions;
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.messagewrappers.BftMessage;
 import org.hyperledger.besu.consensus.common.bft.payload.SignedData;
-import org.hyperledger.besu.consensus.qbft.QbftExtraDataCodec;
 import org.hyperledger.besu.consensus.qbft.payload.PreparePayload;
 import org.hyperledger.besu.consensus.qbft.payload.PreparedRoundMetadata;
 import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload;
@@ -33,7 +33,6 @@
 
 public class RoundChange extends BftMessage<RoundChangePayload> {
 
-  private static final QbftExtraDataCodec QBFT_EXTRA_DATA_ENCODER = new QbftExtraDataCodec();
   private final Optional<Block> proposedBlock;
   private final List<SignedData<PreparePayload>> prepares;
 
@@ -73,7 +72,7 @@ public Bytes encode() {
     return rlpOut.encoded();
   }
 
-  public static RoundChange decode(final Bytes data) {
+  public static RoundChange decode(final Bytes data, final BftExtraDataCodec bftExtraDataCodec) {
 
     final RLPInput rlpIn = RLP.input(data);
     rlpIn.enterList();
@@ -86,8 +85,7 @@ public static RoundChange decode(final Bytes data) {
     } else {
       block =
           Optional.of(
-              Block.readFrom(
-                  rlpIn, BftBlockHeaderFunctions.forCommittedSeal(QBFT_EXTRA_DATA_ENCODER)));
+              Block.readFrom(rlpIn, BftBlockHeaderFunctions.forCommittedSeal(bftExtraDataCodec)));
     }
 
     final List<SignedData<PreparePayload>> prepares =

diff --git a/...ensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/ProposalPayload.java b/...ensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/ProposalPayload.java
@@ -15,8 +15,8 @@
 package org.hyperledger.besu.consensus.qbft.payload;
 
 import org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions;
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier;
-import org.hyperledger.besu.consensus.qbft.QbftExtraDataCodec;
 import org.hyperledger.besu.consensus.qbft.messagedata.QbftV1;
 import org.hyperledger.besu.ethereum.core.Block;
 import org.hyperledger.besu.ethereum.rlp.RLPInput;
@@ -38,12 +38,12 @@ public ProposalPayload(
     this.proposedBlock = proposedBlock;
   }
 
-  public static ProposalPayload readFrom(final RLPInput rlpInput) {
+  public static ProposalPayload readFrom(
+      final RLPInput rlpInput, final BftExtraDataCodec bftExtraDataCodec) {
     rlpInput.enterList();
     final ConsensusRoundIdentifier roundIdentifier = readConsensusRound(rlpInput);
     final Block proposedBlock =
-        Block.readFrom(
-            rlpInput, BftBlockHeaderFunctions.forCommittedSeal(new QbftExtraDataCodec()));
+        Block.readFrom(rlpInput, BftBlockHeaderFunctions.forCommittedSeal(bftExtraDataCodec));
     rlpInput.leaveList();
 
     return new ProposalPayload(roundIdentifier, proposedBlock);

diff --git a/...s/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftController.java b/...s/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftController.java
@@ -14,6 +14,7 @@
  */
 package org.hyperledger.besu.consensus.qbft.statemachine;
 
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.Gossiper;
 import org.hyperledger.besu.consensus.common.bft.MessageTracker;
 import org.hyperledger.besu.consensus.common.bft.SynchronizerUpdater;
@@ -35,6 +36,7 @@ public class QbftController extends BaseBftController {
 
   private BaseQbftBlockHeightManager currentHeightManager;
   private final QbftBlockHeightManagerFactory qbftBlockHeightManagerFactory;
+  private final BftExtraDataCodec bftExtraDataCodec;
 
   public QbftController(
       final Blockchain blockchain,
@@ -43,7 +45,8 @@ public QbftController(
       final Gossiper gossiper,
       final MessageTracker duplicateMessageTracker,
       final FutureMessageBuffer futureMessageBuffer,
-      final SynchronizerUpdater sychronizerUpdater) {
+      final SynchronizerUpdater sychronizerUpdater,
+      final BftExtraDataCodec bftExtraDataCodec) {
 
     super(
         blockchain,
@@ -53,6 +56,7 @@ public QbftController(
         futureMessageBuffer,
         sychronizerUpdater);
     this.qbftBlockHeightManagerFactory = qbftBlockHeightManagerFactory;
+    this.bftExtraDataCodec = bftExtraDataCodec;
   }
 
   @Override
@@ -63,7 +67,7 @@ protected void handleMessage(final Message message) {
       case QbftV1.PROPOSAL:
         consumeMessage(
             message,
-            ProposalMessageData.fromMessageData(messageData).decode(),
+            ProposalMessageData.fromMessageData(messageData).decode(bftExtraDataCodec),
             currentHeightManager::handleProposalPayload);
         break;
 
@@ -84,7 +88,7 @@ protected void handleMessage(final Message message) {
       case QbftV1.ROUND_CHANGE:
         consumeMessage(
             message,
-            RoundChangeMessageData.fromMessageData(messageData).decode(),
+            RoundChangeMessageData.fromMessageData(messageData).decode(bftExtraDataCodec),
             currentHeightManager::handleRoundChangePayload);
         break;
 

diff --git a/...t/src/reference-test/java/org/hyperledger/besu/consensus/qbt/support/ProposalMessage.java b/...t/src/reference-test/java/org/hyperledger/besu/consensus/qbt/support/ProposalMessage.java
@@ -15,6 +15,7 @@
 package org.hyperledger.besu.consensus.qbt.support;
 
 import org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions;
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier;
 import org.hyperledger.besu.consensus.common.bft.messagewrappers.BftMessage;
 import org.hyperledger.besu.consensus.common.bft.payload.SignedData;
@@ -37,6 +38,8 @@
 import org.apache.tuweni.bytes.Bytes;
 
 public class ProposalMessage implements RlpTestCaseMessage {
+  private static final BftExtraDataCodec bftExtraDataCodec = new QbftExtraDataCodec();
+
   private final SignedProposal signedProposal;
   private final List<SignedRoundChange> roundChanges;
 
@@ -54,7 +57,7 @@ public ProposalMessage(
 
   @Override
   public BftMessage<ProposalPayload> fromRlp(final Bytes rlp) {
-    return Proposal.decode(rlp);
+    return Proposal.decode(rlp, bftExtraDataCodec);
   }
 
   @Override

diff --git a/...rc/reference-test/java/org/hyperledger/besu/consensus/qbt/support/RoundChangeMessage.java b/...rc/reference-test/java/org/hyperledger/besu/consensus/qbt/support/RoundChangeMessage.java
@@ -16,6 +16,7 @@
 
 import static org.hyperledger.besu.consensus.common.bft.BftBlockHeaderFunctions.forCommittedSeal;
 
+import org.hyperledger.besu.consensus.common.bft.BftExtraDataCodec;
 import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier;
 import org.hyperledger.besu.consensus.common.bft.messagewrappers.BftMessage;
 import org.hyperledger.besu.consensus.common.bft.payload.SignedData;
@@ -40,6 +41,8 @@
 import org.apache.tuweni.bytes.Bytes;
 
 public class RoundChangeMessage implements RlpTestCaseMessage {
+  private static final BftExtraDataCodec bftExtraDataCodec = new QbftExtraDataCodec();
+
   private final SignedRoundChange signedRoundChange;
   private final Optional<String> block;
 
@@ -57,7 +60,7 @@ public RoundChangeMessage(
 
   @Override
   public BftMessage<RoundChangePayload> fromRlp(final Bytes rlp) {
-    return RoundChange.decode(rlp);
+    return RoundChange.decode(rlp, bftExtraDataCodec);
   }
 
   @Override