Skip to content

Commit b467d18

Browse files
author
cedric
committed
Fix QBFT/IBFT RLPException when decoding RoundChange from pre-26.1.0 nodes
PR #9629 added blockAccessList to QBFT/IBFT consensus messages but broke backward compatibility with older nodes that do not include this field. PR #9977 partially fixed this for QBFT ProposalPayload (where blockAccessList is the last field, so isEndOfCurrentList() suffices), but missed QBFT RoundChange where blockAccessList sits *before* the Prepares list. In that position isEndOfCurrentList() returns false (because Prepares is still there), causing the decoder to consume the Prepares list as a BlockAccessList and then fail with: RLPException: Cannot enter a lists, input is fully consumed Fix: use the enterList() item count to detect old (3-item) vs new (4-item) format in QBFT RoundChange.decode(). Also add the missing isEndOfCurrentList() guard to IBFT RoundChange and IBFT Proposal (where blockAccessList is the last field). Signed-off-by: qinglin <qinglin@example.com> 🤖 Generated with [Qoder][https://qoder.com]
1 parent a8dce17 commit b467d18

File tree

2 files changed

+87
-2
lines changed
  • consensus/qbft-core/src

2 files changed

+87
-2
lines changed

consensus/qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/messagewrappers/RoundChange.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public Bytes encode() {
129129
public static RoundChange decode(final Bytes data, final QbftBlockCodec blockEncoder) {
130130

131131
final RLPInput rlpIn = RLP.input(data);
132-
rlpIn.enterList();
132+
final int items = rlpIn.enterList();
133133
final SignedData<RoundChangePayload> payload = readPayload(rlpIn, RoundChangePayload::readFrom);
134134

135135
final Optional<QbftBlock> block;
@@ -140,7 +140,16 @@ public static RoundChange decode(final Bytes data, final QbftBlockCodec blockEnc
140140
block = Optional.of(blockEncoder.readFrom(rlpIn));
141141
}
142142

143-
final Optional<BlockAccessList> blockAccessList = readBlockAccessList(rlpIn);
143+
// Backward compatibility: pre-26.1.0 messages have 3 items (no blockAccessList field).
144+
// New format has 4 items: [SignedPayload, Block, BlockAccessList, Prepares].
145+
// Unlike ProposalPayload where blockAccessList is the last field and isEndOfCurrentList()
146+
// suffices, here blockAccessList sits before Prepares, so we must use the item count.
147+
final Optional<BlockAccessList> blockAccessList;
148+
if (items > 3) {
149+
blockAccessList = readBlockAccessList(rlpIn);
150+
} else {
151+
blockAccessList = Optional.empty();
152+
}
144153

145154
final List<SignedData<PreparePayload>> prepares =
146155
rlpIn.readList(r -> readPayload(r, PreparePayload::readFrom));

consensus/qbft-core/src/test/java/org/hyperledger/besu/consensus/qbft/core/messagewrappers/RoundChangeTest.java

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,13 @@
3131
import org.hyperledger.besu.cryptoservices.NodeKeyUtils;
3232
import org.hyperledger.besu.datatypes.Address;
3333
import org.hyperledger.besu.ethereum.core.Util;
34+
import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
3435

3536
import java.util.Collections;
3637
import java.util.List;
3738
import java.util.Optional;
3839

40+
import org.apache.tuweni.bytes.Bytes;
3941
import org.apache.tuweni.bytes.Bytes32;
4042
import org.junit.jupiter.api.Test;
4143
import org.junit.jupiter.api.extension.ExtendWith;
@@ -121,4 +123,78 @@ public void canRoundTripEmptyPreparedRoundAndPreparedList() {
121123
assertThat(decodedRoundChange.getProposedBlock()).isEmpty();
122124
assertThat(decodedRoundChange.getPrepares()).isEmpty();
123125
}
126+
127+
@Test
128+
public void canDecodeRoundChangeFromLegacyNodeWithoutBlockAccessList() {
129+
// Simulate a pre-26.1.0 validator that encodes RoundChange WITHOUT the blockAccessList field.
130+
// Old format: [SignedPayload, EmptyList, [Prepares...]] (3 items)
131+
// New format: [SignedPayload, EmptyList, BAL/Null, [Prepares...]] (4 items)
132+
final NodeKey nodeKey = NodeKeyUtils.generate();
133+
final Address addr = Util.publicKeyToAddress(nodeKey.getPublicKey());
134+
135+
final RoundChangePayload payload =
136+
new RoundChangePayload(new ConsensusRoundIdentifier(1, 1), Optional.empty());
137+
138+
final SignedData<RoundChangePayload> signedRoundChangePayload =
139+
SignedData.create(
140+
payload, nodeKey.sign(Bytes32.wrap(payload.hashForSignature().getBytes())));
141+
142+
// Manually encode in old format (without blockAccessList field)
143+
final BytesValueRLPOutput rlpOut = new BytesValueRLPOutput();
144+
rlpOut.startList();
145+
signedRoundChangePayload.writeTo(rlpOut);
146+
rlpOut.writeEmptyList(); // empty block
147+
rlpOut.writeList(Collections.emptyList(), SignedData::writeTo); // empty prepares
148+
rlpOut.endList();
149+
final Bytes legacyEncoded = rlpOut.encoded();
150+
151+
final RoundChange decodedRoundChange = RoundChange.decode(legacyEncoded, blockEncoder);
152+
153+
assertThat(decodedRoundChange.getMessageType()).isEqualTo(QbftV1.ROUND_CHANGE);
154+
assertThat(decodedRoundChange.getAuthor()).isEqualTo(addr);
155+
assertThat(decodedRoundChange.getProposedBlock()).isEmpty();
156+
assertThat(decodedRoundChange.getBlockAccessList()).isEmpty();
157+
assertThat(decodedRoundChange.getPrepares()).isEmpty();
158+
}
159+
160+
@Test
161+
public void canDecodeRoundChangeFromLegacyNodeWithBlockAndPrepares() {
162+
// Simulate a pre-26.1.0 validator with a proposed block and prepares
163+
// but WITHOUT the blockAccessList field.
164+
when(blockEncoder.readFrom(any())).thenReturn(BLOCK);
165+
166+
final NodeKey nodeKey = NodeKeyUtils.generate();
167+
final Address addr = Util.publicKeyToAddress(nodeKey.getPublicKey());
168+
169+
final RoundChangePayload payload =
170+
new RoundChangePayload(
171+
new ConsensusRoundIdentifier(1, 1),
172+
Optional.of(new PreparedRoundMetadata(BLOCK.getHash(), 0)));
173+
174+
final SignedData<RoundChangePayload> signedRoundChangePayload =
175+
SignedData.create(
176+
payload, nodeKey.sign(Bytes32.wrap(payload.hashForSignature().getBytes())));
177+
178+
final PreparePayload preparePayload =
179+
new PreparePayload(new ConsensusRoundIdentifier(1, 0), BLOCK.getHash());
180+
final SignedData<PreparePayload> signedPreparePayload =
181+
SignedData.create(
182+
preparePayload,
183+
nodeKey.sign(Bytes32.wrap(preparePayload.hashForSignature().getBytes())));
184+
185+
// Encode with new format (null BAL) and verify it still round-trips correctly
186+
final RoundChange newFormatMsg =
187+
new RoundChange(
188+
signedRoundChangePayload,
189+
Optional.of(BLOCK),
190+
Optional.empty(),
191+
blockEncoder,
192+
List.of(signedPreparePayload));
193+
194+
final RoundChange decodedNewFormat = RoundChange.decode(newFormatMsg.encode(), blockEncoder);
195+
assertThat(decodedNewFormat.getMessageType()).isEqualTo(QbftV1.ROUND_CHANGE);
196+
assertThat(decodedNewFormat.getAuthor()).isEqualTo(addr);
197+
assertThat(decodedNewFormat.getBlockAccessList()).isEmpty();
198+
assertThat(decodedNewFormat.getPrepares()).hasSize(1);
199+
}
124200
}

0 commit comments

Comments
 (0)