Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented expectedGasLimit from spec #8909

Merged
merged 6 commits into from
Dec 16, 2024
Merged

Implemented expectedGasLimit from spec #8909

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6f870b6
Implemented expectedGasLimit from spec
rolfyone Dec 11, 2024
8e6a4cb
changelog
rolfyone Dec 11, 2024
78060db
typo
rolfyone Dec 11, 2024
71620bd
Merge branch 'master' into builder-gas-computation
rolfyone Dec 11, 2024
e8c4857
Merge branch 'master' into builder-gas-computation
rolfyone Dec 12, 2024
7933623
Merge branch 'master' into builder-gas-computation
rolfyone Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@
- Set `is_syncing` to `false` instead of `true` for the `/eth/v1/node/syncing` API endpoint when the head is optimistic and the sync distance is 0
- Fix libp2p direct peers handling
- Added check for gossip message maximum uncompressed size
- Updated the gas change check for block building so that warnings only get raised if the change is off spec.
25 changes: 13 additions & 12 deletions ...ayer/src/main/java/tech/pegasys/teku/ethereum/executionlayer/BuilderBidValidatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,20 +122,21 @@ public void validateBuilderBid(
final UInt64 preferredGasLimit = validatorRegistration.getGasLimit();
final UInt64 proposedGasLimit = executionPayloadHeader.getGasLimit();

if (parentGasLimit.equals(preferredGasLimit) && proposedGasLimit.equals(parentGasLimit)) {
return;
}

if (preferredGasLimit.isGreaterThan(parentGasLimit)
&& proposedGasLimit.isGreaterThan(parentGasLimit)) {
return;
final UInt64 expectedGasLimit = expectedGasLimit(parentGasLimit, preferredGasLimit);
if (!expectedGasLimit.equals(preferredGasLimit)) {
eventLogger.builderBidNotHonouringGasLimit(
parentGasLimit, proposedGasLimit, preferredGasLimit);
}
}

if (preferredGasLimit.isLessThan(parentGasLimit)
&& proposedGasLimit.isLessThan(parentGasLimit)) {
return;
static UInt64 expectedGasLimit(final UInt64 parentGasLimit, final UInt64 targetGasLimit) {
final UInt64 maxGasLimitDifference = parentGasLimit.dividedBy(1024L).minusMinZero(1);
rolfyone marked this conversation as resolved.
Show resolved Hide resolved
if (targetGasLimit.isGreaterThan(parentGasLimit)) {
final UInt64 gasDiff = targetGasLimit.minusMinZero(parentGasLimit);
return parentGasLimit.plus(maxGasLimitDifference.min(gasDiff));
} else {
final UInt64 gasDiff = parentGasLimit.minusMinZero(targetGasLimit);
return parentGasLimit.minusMinZero(maxGasLimitDifference.min(gasDiff));
}

eventLogger.builderBidNotHonouringGasLimit(parentGasLimit, proposedGasLimit, preferredGasLimit);
}
}
49 changes: 41 additions & 8 deletions ...ayer/src/test/java/tech/pegasys/teku/ethereum/executionlayer/BuilderBidValidatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

package tech.pegasys.teku.ethereum.executionlayer;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doNothing;
Expand All @@ -24,12 +25,16 @@
import static tech.pegasys.teku.spec.schemas.ApiSchemas.VALIDATOR_REGISTRATION_SCHEMA;

import java.util.Optional;
import java.util.stream.Stream;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.bytes.Bytes32;
import org.apache.tuweni.units.bigints.UInt256;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import tech.pegasys.teku.bls.BLS;
import tech.pegasys.teku.bls.BLSConstants;
import tech.pegasys.teku.bls.BLSKeyPair;
Expand Down Expand Up @@ -148,10 +153,21 @@ void shouldBeInvalidIfLocalPayloadWithdrawalsRootDoesNotMatchTheRootOfTheBid() {
.orElseThrow());
}

@ParameterizedTest(name = "parent.{0}_target.{1}_result.{2}")
@MethodSource("expectedGasLimitPermutations")
void expectedGasLimitTestCases(
final long parentGasLimit, final long targetGasLimit, final long expectedGasLimit) {
final UInt64 computedGasLimit =
BuilderBidValidatorImpl.expectedGasLimit(
UInt64.valueOf(parentGasLimit), UInt64.valueOf(targetGasLimit));

assertThat(computedGasLimit).isEqualTo(UInt64.valueOf(expectedGasLimit));
}

@Test
void shouldNotLogEventIfGasLimitDecreases() throws BuilderBidValidationException {

prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1022_000), UInt64.valueOf(1023_000));
// 1023001 is as high as it can move in 1 shot
prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1022_000), UInt64.valueOf(1023_001));

builderBidValidatorWithMockSpec.validateBuilderBid(
signedBuilderBid, validatorRegistration, state, Optional.empty());
Expand All @@ -161,8 +177,8 @@ void shouldNotLogEventIfGasLimitDecreases() throws BuilderBidValidationException

@Test
void shouldNotLogEventIfGasLimitIncreases() throws BuilderBidValidationException {

prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1025_000), UInt64.valueOf(2048_000));
// 1024999 is as high as it can move in 1 shot
prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1025_000), UInt64.valueOf(1024_999));

builderBidValidatorWithMockSpec.validateBuilderBid(
signedBuilderBid, validatorRegistration, state, Optional.empty());
Expand All @@ -184,27 +200,44 @@ void shouldNotLogEventIfGasLimitStaysTheSame() throws BuilderBidValidationExcept
@Test
void shouldLogEventIfGasLimitDoesNotDecrease() throws BuilderBidValidationException {

prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1024_000), UInt64.valueOf(1023_100));
prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1024_000), UInt64.valueOf(1020_000));

builderBidValidatorWithMockSpec.validateBuilderBid(
signedBuilderBid, validatorRegistration, state, Optional.empty());

verify(eventLogger)
.builderBidNotHonouringGasLimit(
UInt64.valueOf(1024_000), UInt64.valueOf(1024_000), UInt64.valueOf(1023_100));
UInt64.valueOf(1024_000), UInt64.valueOf(1024_000), UInt64.valueOf(1020_000));
}

@Test
void shouldLogEventIfGasLimitDoesNotIncrease() throws BuilderBidValidationException {

prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1020_000), UInt64.valueOf(1024_100));
prepareGasLimit(UInt64.valueOf(1024_000), UInt64.valueOf(1020_000), UInt64.valueOf(1028_000));

builderBidValidatorWithMockSpec.validateBuilderBid(
signedBuilderBid, validatorRegistration, state, Optional.empty());

verify(eventLogger)
.builderBidNotHonouringGasLimit(
UInt64.valueOf(1024_000), UInt64.valueOf(1020_000), UInt64.valueOf(1024_100));
UInt64.valueOf(1024_000), UInt64.valueOf(1020_000), UInt64.valueOf(1028_000));
}

static Stream<Arguments> expectedGasLimitPermutations() {
return Stream.of(
// same, expect no change
Arguments.of(36_000_000L, 36_000_000L, 36_000_000L),
Arguments.of(1024_000L, 1024_000L, 1024_000L),
// down inside bounds
Arguments.of(1024_000L, 1023_500L, 1023_500L),
// down outside bounds - results in a partial move
Arguments.of(36_000_000L, 35_000_000L, 35_964_845L),
Arguments.of(1024_000L, 1020_000L, 1023_001L),
// increase outside bounds - results in a partial move
Arguments.of(1024_000L, 1025_000L, 1024_999L),
Arguments.of(30_000_000L, 36_000_000L, 30_029_295L),
// increase inside bounds
Arguments.of(1024_000L, 1024_500L, 1024_500L));
Comment on lines +229 to +240
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jtraglia so the args are parent, target, expected output...

}

private void prepareValidSignedBuilderBid() {
Expand Down
Loading