Skip to content

Commit c590df1

Browse files
mehdi-aouadimehdi-aouadi
committed
fixes
1 parent a382bad commit c590df1

File tree

4 files changed

+94
-18
lines changed

4 files changed

+94
-18
lines changed

‎ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/execution/DefaultExecutionPayloadBidManager.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ public SafeFuture<InternalValidationResult> validateAndAddBid(
5757
validationResult.thenAccept(
5858
result -> {
5959
switch (result.code()) {
60-
case ACCEPT -> {}
61-
case REJECT, SAVE_FOR_FUTURE, IGNORE -> {}
60+
// TODO-GLOAS handle bids
61+
case ACCEPT, REJECT, SAVE_FOR_FUTURE, IGNORE -> {}
6262
}
6363
});
6464
return validationResult;

‎ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/ExecutionPayloadBidGossipValidator.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,26 +90,22 @@ public SafeFuture<InternalValidationResult> validate(
9090
LOG.trace(
9191
"Already received a bid with a higher value {} for block with parent hash {}. Current bid's value is {}",
9292
existingBidValue,
93-
bid.getBlockHash(),
93+
bid.getParentBlockHash(),
9494
bid.getValue());
9595
return completedFuture(
9696
ignore(
9797
"Already received a bid with a higher value %s for block with parent hash %s. Current bid's value is %s",
9898
existingBidValue, bid.getParentBlockHash(), bid.getValue()));
99-
} else {
100-
highestBids.put(bid.getParentBlockHash(), bid.getValue());
10199
}
102-
} else {
103-
highestBids.put(bid.getParentBlockHash(), bid.getValue());
104100
}
105101

106102
/*
107103
* [IGNORE] bid.slot is the current slot or the next slot.
108104
*/
109105

110-
// This check take the gossip clock disparity allowance and hence accepts bids with current
106+
// This check considers the gossip clock disparity allowance and hence accepts bids with current
111107
// slot, next slot but not too early, previous slot but not too late
112-
if (!gossipValidationHelper.isSlotWithinGossipTimeWindow(bid.getSlot())) {
108+
if (!gossipValidationHelper.isCurrentSlotWithGossipDisparityAllowance(bid.getSlot())) {
113109
LOG.trace("Bid must be for current or next slot but was for slot {}", bid.getSlot());
114110
return completedFuture(
115111
ignore("Bid must be for current or next slot but was for slot %s", bid.getSlot()));
@@ -201,6 +197,7 @@ public SafeFuture<InternalValidationResult> validate(
201197
LOG.trace("Invalid payload execution bid signature");
202198
return reject("Invalid payload execution bid signature");
203199
}
200+
highestBids.put(bid.getParentBlockHash(), bid.getValue());
204201
seenExecutionPayloadBids.add(key);
205202
return ACCEPT;
206203
});

‎ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/GossipValidationHelper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ public boolean isValidBuilderIndex(
154154
}
155155
final Validator builder = state.getValidators().get(index);
156156
final boolean isActiveBuilder =
157-
spec.getActiveValidatorIndices(state, slot).contains(builderIndex.intValue());
157+
spec.getActiveValidatorIndices(state, spec.computeEpochAtSlot(slot))
158+
.contains(builderIndex.intValue());
158159
return !builder.isSlashed() && isActiveBuilder;
159160
}
160161

‎ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/ExecutionPayloadBidGossipValidatorTest.java‎

Lines changed: 86 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import static org.mockito.ArgumentMatchers.eq;
1818
import static org.mockito.Mockito.clearInvocations;
1919
import static org.mockito.Mockito.mock;
20-
import static org.mockito.Mockito.never;
2120
import static org.mockito.Mockito.verify;
2221
import static org.mockito.Mockito.when;
2322
import static tech.pegasys.teku.infrastructure.async.SafeFutureAssert.assertThatSafeFuture;
@@ -71,7 +70,7 @@ void setup(final TestSpecInvocationContextProvider.SpecContext specContext) {
7170
parentBlockHash = bid.getParentBlockHash();
7271
postState = dataStructureUtil.randomBeaconState();
7372

74-
when(gossipValidationHelper.isSlotWithinGossipTimeWindow(slot)).thenReturn(true);
73+
when(gossipValidationHelper.isCurrentSlotWithGossipDisparityAllowance(slot)).thenReturn(true);
7574
when(gossipValidationHelper.isBlockHashKnown(parentBlockHash, parentBlockRoot))
7675
.thenReturn(true);
7776
when(gossipValidationHelper.getSlotForBlockRoot(parentBlockRoot))
@@ -116,7 +115,7 @@ void shouldReject_whenExecutionPaymentIsNonZero() {
116115

117116
@TestTemplate
118117
void shouldIgnore_whenSlotIsNotWithinGossipWindow() {
119-
when(gossipValidationHelper.isSlotWithinGossipTimeWindow(slot)).thenReturn(false);
118+
when(gossipValidationHelper.isCurrentSlotWithGossipDisparityAllowance(slot)).thenReturn(false);
120119
assertThatSafeFuture(bidValidator.validate(signedBid))
121120
.isCompletedWithValue(
122121
ignore("Bid must be for current or next slot but was for slot %s", slot));
@@ -155,6 +154,89 @@ void shouldIgnore_whenBidValueIsLowerThanSeen() {
155154
bid.getValue(), parentBlockHash, lowerBidValue));
156155
}
157156

157+
@TestTemplate
158+
void shouldNotCacheHigherBidIfInvalid() {
159+
// valid, lower value bid is accepted and cached
160+
final UInt64 lowerValue = signedBid.getMessage().getValue();
161+
assertThatSafeFuture(bidValidator.validate(signedBid)).isCompletedWithValue(ACCEPT);
162+
163+
// modified copy of the original bid with a higher value and different builder index
164+
final UInt64 higherValue = lowerValue.plus(10);
165+
final UInt64 differentBuilderIndex = builderIndex.plus(1);
166+
final ExecutionPayloadBid originalBidMessage = signedBid.getMessage();
167+
final ExecutionPayloadBid higherValueBidMessage =
168+
originalBidMessage
169+
.getSchema()
170+
.create(
171+
originalBidMessage.getParentBlockHash(),
172+
originalBidMessage.getParentBlockRoot(),
173+
originalBidMessage.getBlockHash(),
174+
originalBidMessage.getPrevRandao(),
175+
originalBidMessage.getFeeRecipient(),
176+
originalBidMessage.getGasLimit(),
177+
differentBuilderIndex,
178+
originalBidMessage.getSlot(),
179+
higherValue,
180+
originalBidMessage.getExecutionPayment(),
181+
originalBidMessage.getBlobKzgCommitmentsRoot());
182+
183+
final SignedExecutionPayloadBid higherValueInvalidBid =
184+
dataStructureUtil.randomSignedExecutionPayloadBid(higherValueBidMessage);
185+
186+
when(gossipValidationHelper.isValidBuilderIndex(differentBuilderIndex, postState, slot))
187+
.thenReturn(true);
188+
when(gossipValidationHelper.builderHasEnoughBalanceForBid(
189+
higherValue, differentBuilderIndex, postState, slot))
190+
.thenReturn(true);
191+
when(gossipValidationHelper.hasBuilderWithdrawalCredential(
192+
differentBuilderIndex, postState, slot))
193+
.thenReturn(true);
194+
// bad signature to make it fail validation
195+
when(gossipValidationHelper.isSignatureValidWithRespectToBuilderIndex(
196+
any(), eq(higherValueInvalidBid.getMessage().getBuilderIndex()), any(), any()))
197+
.thenReturn(false);
198+
// invalid, higher value bid is rejected
199+
assertThatSafeFuture(bidValidator.validate(higherValueInvalidBid))
200+
.isCompletedWithValue(reject("Invalid payload execution bid signature"));
201+
202+
// valid, intermediate value bid with a higher value that the initially cached one
203+
final UInt64 intermediateValue = lowerValue.plus(5);
204+
final UInt64 intermediateBidBuilderIndex = builderIndex.plus(2);
205+
final SignedExecutionPayloadBid intermediateValueValidBid =
206+
dataStructureUtil.randomSignedExecutionPayloadBid(
207+
dataStructureUtil.randomExecutionPayloadBid(
208+
parentBlockHash,
209+
slot,
210+
intermediateBidBuilderIndex,
211+
intermediateValue,
212+
UInt64.ZERO));
213+
214+
when(gossipValidationHelper.isBlockHashKnown(
215+
parentBlockHash, intermediateValueValidBid.getMessage().getParentBlockRoot()))
216+
.thenReturn(true);
217+
when(gossipValidationHelper.isValidBuilderIndex(intermediateBidBuilderIndex, postState, slot))
218+
.thenReturn(true);
219+
when(gossipValidationHelper.getSlotForBlockRoot(
220+
intermediateValueValidBid.getMessage().getParentBlockRoot()))
221+
.thenReturn(Optional.of(slot));
222+
when(gossipValidationHelper.builderHasEnoughBalanceForBid(
223+
intermediateValue, intermediateBidBuilderIndex, postState, slot))
224+
.thenReturn(true);
225+
when(gossipValidationHelper.getParentStateInBlockEpoch(
226+
slot, intermediateValueValidBid.getMessage().getParentBlockRoot(), slot))
227+
.thenReturn(SafeFuture.completedFuture(Optional.of(postState)));
228+
when(gossipValidationHelper.hasBuilderWithdrawalCredential(
229+
intermediateBidBuilderIndex, postState, slot))
230+
.thenReturn(true);
231+
when(gossipValidationHelper.isSignatureValidWithRespectToBuilderIndex(
232+
any(), eq(intermediateValueValidBid.getMessage().getBuilderIndex()), any(), any()))
233+
.thenReturn(true);
234+
235+
// the intermediate value bid is accepted
236+
assertThatSafeFuture(bidValidator.validate(intermediateValueValidBid))
237+
.isCompletedWithValue(ACCEPT);
238+
}
239+
158240
@TestTemplate
159241
void shouldSaveForFuture_whenParentBlockHashIsUnknown() {
160242
when(gossipValidationHelper.isBlockHashKnown(parentBlockHash, parentBlockRoot))
@@ -217,7 +299,7 @@ void shouldReject_whenSignatureIsInvalid() {
217299
}
218300

219301
@TestTemplate
220-
void shouldSkipExpensiveChecksForSeenBid() {
302+
void shouldIgnoreSeenBid() {
221303
assertThatSafeFuture(bidValidator.validate(signedBid)).isCompletedWithValue(ACCEPT);
222304
verify(gossipValidationHelper).isBlockHashKnown(parentBlockHash, parentBlockRoot);
223305
verify(gossipValidationHelper).getParentStateInBlockEpoch(any(), any(), any());
@@ -230,10 +312,6 @@ void shouldSkipExpensiveChecksForSeenBid() {
230312
ignore(
231313
"Already received a bid from builder with index %s at slot %s",
232314
builderIndex, slot));
233-
verify(gossipValidationHelper, never()).isBlockHashKnown(any(), any());
234-
verify(gossipValidationHelper, never()).getParentStateInBlockEpoch(any(), any(), any());
235-
verify(gossipValidationHelper, never())
236-
.isSignatureValidWithRespectToBuilderIndex(any(), any(), any(), any());
237315
}
238316

239317
@TestTemplate

0 commit comments

Comments
 (0)