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

Change skip_randao_verification param to string #8338

Merged
merged 18 commits into from
Jun 6, 2024
Merged

Change skip_randao_verification param to string #8338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
7004354
change query param from boolean to string
gfukushima May 8, 2024
daeabe1
fix test
gfukushima May 8, 2024
a522f7d
Merge branch 'refs/heads/master' into skip-randao-string
gfukushima May 27, 2024
21e98ef
Merge branch 'refs/heads/master' into skip-randao-string
gfukushima May 27, 2024
35600b5
Add flag style string query parameter
gfukushima May 27, 2024
0fa52b1
Add queryParams that explicitly allow empty values
gfukushima May 27, 2024
3401cff
adjust test
gfukushima May 27, 2024
6ec7d32
remove example
gfukushima May 27, 2024
12e172d
fix test
gfukushima May 27, 2024
73cfe23
extract validation into separate method
gfukushima May 29, 2024
035b248
apply intelliJ suggestion
gfukushima May 29, 2024
8f29351
Change skip_randao_verification description to inform that flag is cu…
gfukushima May 30, 2024
a88f5f4
Merge branch 'master' into skip-randao-string
gfukushima May 30, 2024
52fdec6
Change skip_randao_verification param description to clarify teku doe…
gfukushima May 31, 2024
2883e18
Merge remote-tracking branch 'origin/skip-randao-string' into skip-ra…
gfukushima May 31, 2024
efa5b69
Merge branch 'master' into skip-randao-string
gfukushima May 31, 2024
6278f89
Merge branch 'master' into skip-randao-string
gfukushima Jun 3, 2024
683724a
Merge branch 'master' into skip-randao-string
gfukushima Jun 6, 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
17 changes: 10 additions & 7 deletions ...sources/tech/pegasys/teku/beaconrestapi/beacon/paths/_eth_v3_validator_blocks_{slot}.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,6 @@
"example" : "0xcf8e0d4e9587369b2301d0790347320302cc0943d5a1884560367e8208d920f2",
"format" : "byte"
}
}, {
"name" : "skip_randao_verification",
"in" : "query",
"schema" : {
"type" : "boolean",
"description" : "Skip verification of the `randao_reveal` value. If this flag is set then the\n `randao_reveal` must be set to the point at infinity (`0xc0..00`). This query parameter\n is a flag and does not take a value"
}
}, {
"name" : "builder_boost_factor",
"in" : "query",
Expand All @@ -49,6 +42,16 @@
"example" : "1",
"format" : "uint64"
}
},{
"name" : "skip_randao_verification",
"allowEmptyValue" : true,
"in" : "query",
"schema" : {
"type" : "string",
"description" : "Skip verification of the `randao_reveal` value. Ignored in the Teku implementation.",
"minLength" : 0,
"maxLength" : 0
}
} ],
"responses" : {
"200" : {
Expand Down
4 changes: 2 additions & 2 deletions data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/BeaconRestApiTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,10 @@ public class BeaconRestApiTypes {
SIGNATURE_TYPE.withDescription(
"`BLSSignature Hex` BLS12-381 signature for the current epoch."));

public static final ParameterMetadata<Boolean> SKIP_RANDAO_VERIFICATION_PARAMETER =
public static final ParameterMetadata<String> SKIP_RANDAO_VERIFICATION_PARAMETER =
new ParameterMetadata<>(
RestApiConstants.SKIP_RANDAO_VERIFICATION,
BOOLEAN_TYPE.withDescription(SKIP_RANDAO_VERIFICATION_PARAM_DESCRIPTION));
CoreTypes.flag(SKIP_RANDAO_VERIFICATION_PARAM_DESCRIPTION));

public static final ParameterMetadata<UInt64> BUILDER_BOOST_FACTOR_PARAMETER =
new ParameterMetadata<>(
Expand Down
2 changes: 1 addition & 1 deletion ...pi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v3/validator/GetNewBlockV3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private static EndpointMetadata getEndpointMetaData(
.pathParam(SLOT_PARAMETER.withDescription(SLOT_PATH_DESCRIPTION))
.queryParamRequired(RANDAO_PARAMETER)
.queryParam(GRAFFITI_PARAMETER)
.queryParam(SKIP_RANDAO_VERIFICATION_PARAMETER)
.queryParamAllowsEmpty(SKIP_RANDAO_VERIFICATION_PARAMETER)
.queryParam(BUILDER_BOOST_FACTOR_PARAMETER)
.response(
SC_OK,
Expand Down
4 changes: 1 addition & 3 deletions ...astructure/http/src/main/java/tech/pegasys/teku/infrastructure/http/RestApiConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,7 @@ public class RestApiConstants {
+ SC_SERVICE_UNAVAILABLE
+ " status code if set to true and no validators have been registered with the builder";
public static final String SKIP_RANDAO_VERIFICATION_PARAM_DESCRIPTION =
"Skip verification of the `randao_reveal` value. If this flag is set then the\n"
+ " `randao_reveal` must be set to the point at infinity (`0xc0..00`). This query parameter\n"
+ " is a flag and does not take a value";
"Skip verification of the `randao_reveal` value. Ignored in the Teku implementation.";

public static final String BUILDER_BOOST_FACTOR = "builder_boost_factor";
public static final String BUILDER_BOOST_FACTOR_DESCRIPTION =
Expand Down
4 changes: 4 additions & 0 deletions infrastructure/json/src/main/java/tech/pegasys/teku/infrastructure/json/types/CoreTypes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ public static StringValueTypeDefinition<String> string(
return stringBuilder().description(description).example(example).build();
}

public static StringValueTypeDefinition<String> flag(final String description) {
return stringBuilder().description(description).minLength(0).maxLength(0).build();
}

private static StringTypeBuilder<String> stringBuilder() {
return DeserializableTypeDefinition.string(String.class)
.formatter(Function.identity())
Expand Down
49 changes: 44 additions & 5 deletions .../java/tech/pegasys/teku/infrastructure/json/types/StringBasedPrimitiveTypeDefinition.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public class StringBasedPrimitiveTypeDefinition<T> implements StringValueTypeDef
private final Optional<String> example;
private final Optional<String> format;
private final Optional<String> pattern;
private final Optional<Integer> minLength;
private final Optional<Integer> maxLength;

private StringBasedPrimitiveTypeDefinition(
final Optional<String> name,
Expand All @@ -43,7 +45,9 @@ private StringBasedPrimitiveTypeDefinition(
final Optional<String> example,
final Optional<String> description,
final Optional<String> format,
final Optional<String> pattern) {
final Optional<String> pattern,
final Optional<Integer> minLength,
final Optional<Integer> maxLength) {
this.name = name;
this.title = title;
this.objectFromString = objectFromString;
Expand All @@ -52,6 +56,8 @@ private StringBasedPrimitiveTypeDefinition(
this.description = description;
this.format = format;
this.pattern = pattern;
this.minLength = minLength;
this.maxLength = maxLength;
}

@Override
Expand All @@ -78,7 +84,9 @@ public StringValueTypeDefinition<T> withDescription(final String description) {
example,
Optional.of(description),
format,
pattern);
pattern,
minLength,
maxLength);
}

@Override
Expand All @@ -105,6 +113,12 @@ public void serializeOpenApiType(final JsonGenerator gen) throws IOException {
if (format.isPresent()) {
gen.writeStringField("format", format.get());
}
if (minLength.isPresent()) {
gen.writeNumberField("minLength", minLength.get());
}
if (maxLength.isPresent()) {
gen.writeNumberField("maxLength", maxLength.get());
}
rolfyone marked this conversation as resolved.
Show resolved Hide resolved
gen.writeEndObject();
}

Expand All @@ -117,6 +131,8 @@ public String toString() {
.add("example", example)
.add("format", format)
.add("pattern", pattern)
.add("minLength", minLength)
.add("maxLength", maxLength)
.toString();
}

Expand Down Expand Up @@ -144,6 +160,8 @@ public static class StringTypeBuilder<T> {
private Optional<String> description = Optional.empty();
private Optional<String> format = Optional.empty();
private Optional<String> pattern = Optional.empty();
private Optional<Integer> minLength = Optional.empty();
private Optional<Integer> maxLength = Optional.empty();

public StringTypeBuilder<T> name(final String name) {
this.name = Optional.of(name);
Expand Down Expand Up @@ -185,12 +203,31 @@ public StringTypeBuilder<T> pattern(final String pattern) {
return this;
}

public StringTypeBuilder<T> minLength(final int minLength) {
this.minLength = Optional.of(minLength);
return this;
}

public StringTypeBuilder<T> maxLength(final int maxLength) {
this.maxLength = Optional.of(maxLength);
return this;
}

public StringValueTypeDefinition<T> build() {
checkNotNull(parser, "Must specify parser");
checkNotNull(formatter, "Must specify formatter");

return new StringBasedPrimitiveTypeDefinition<>(
name, title.or(() -> name), parser, formatter, example, description, format, pattern);
name,
title.or(() -> name),
parser,
formatter,
example,
description,
format,
pattern,
minLength,
maxLength);
}
}

Expand All @@ -208,11 +245,13 @@ public boolean equals(final Object o) {
&& Objects.equals(description, that.description)
&& Objects.equals(example, that.example)
&& Objects.equals(format, that.format)
&& Objects.equals(pattern, that.pattern);
&& Objects.equals(pattern, that.pattern)
&& Objects.equals(minLength, that.minLength)
&& Objects.equals(maxLength, that.maxLength);
}

@Override
public int hashCode() {
return Objects.hash(name, title, description, example, format, pattern);
return Objects.hash(name, title, description, example, format, pattern, minLength, maxLength);
}
}
71 changes: 49 additions & 22 deletions ...pi/src/main/java/tech/pegasys/teku/infrastructure/restapi/endpoints/EndpointMetadata.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public class EndpointMetadata {
private final Map<String, StringValueTypeDefinition<?>> pathParams;
private final Map<String, StringValueTypeDefinition<?>> requiredQueryParams;
private final Map<String, StringValueTypeDefinition<?>> queryParams;
private final Map<String, StringValueTypeDefinition<?>> queryParamsAllowEmpty;
private final Map<String, StringValueTypeDefinition<?>> queryListParams;
private final Map<String, StringValueTypeDefinition<?>> requiredHeaders;
private final Map<String, StringValueTypeDefinition<?>> headers;
Expand All @@ -105,6 +106,7 @@ private EndpointMetadata(
final List<String> tags,
final Map<String, StringValueTypeDefinition<?>> pathParams,
final Map<String, StringValueTypeDefinition<?>> queryParams,
final Map<String, StringValueTypeDefinition<?>> queryParamsAllowEmpty,
final Map<String, StringValueTypeDefinition<?>> requiredQueryParams,
final Map<String, StringValueTypeDefinition<?>> queryListParams,
final Map<String, StringValueTypeDefinition<?>> requiredHeaders,
Expand All @@ -124,6 +126,7 @@ private EndpointMetadata(
this.tags = tags;
this.pathParams = pathParams;
this.queryParams = queryParams;
this.queryParamsAllowEmpty = queryParamsAllowEmpty;
this.requiredQueryParams = requiredQueryParams;
this.queryListParams = queryListParams;
this.requiredHeaders = requiredHeaders;
Expand Down Expand Up @@ -223,12 +226,15 @@ public StringValueTypeDefinition<?> getQueryParameterDefinition(final String par
checkArgument(
requiredQueryParams.containsKey(parameterName)
|| queryParams.containsKey(parameterName)
|| queryParamsAllowEmpty.containsKey(parameterName)
|| queryListParams.containsKey(parameterName),
"Query parameter " + parameterName + " was not found in endpoint metadata");
if (requiredQueryParams.containsKey(parameterName)) {
return requiredQueryParams.get(parameterName);
} else if (queryParams.containsKey(parameterName)) {
return queryParams.get(parameterName);
} else if (queryParamsAllowEmpty.containsKey(parameterName)) {
return queryParamsAllowEmpty.get(parameterName);
}

return queryListParams.get(parameterName);
Expand Down Expand Up @@ -308,19 +314,21 @@ public void writeOpenApi(final JsonGenerator gen) throws IOException {
if (deprecated) {
gen.writeBooleanField("deprecated", true);
}
if (pathParams.size() > 0
|| queryParams.size() > 0
|| requiredQueryParams.size() > 0
|| queryListParams.size() > 0
|| requiredHeaders.size() > 0
|| headers.size() > 0) {
if (!pathParams.isEmpty()
|| !queryParams.isEmpty()
|| !queryParamsAllowEmpty.isEmpty()
|| !requiredQueryParams.isEmpty()
|| !queryListParams.isEmpty()
|| !requiredHeaders.isEmpty()
|| !headers.isEmpty()) {
gen.writeArrayFieldStart("parameters");
writeParameters(gen, pathParams, "path", true, false);
writeParameters(gen, requiredQueryParams, "query", true, false);
writeParameters(gen, queryParams, "query", false, false);
writeParameters(gen, queryListParams, "query", false, true);
writeParameters(gen, requiredHeaders, "header", true, false);
writeParameters(gen, headers, "header", false, false);
writeParameters(gen, pathParams, "path", true, false, false);
writeParameters(gen, requiredQueryParams, "query", true, false, false);
writeParameters(gen, queryParams, "query", false, false, false);
writeParameters(gen, queryParamsAllowEmpty, "query", false, false, true);
writeParameters(gen, queryListParams, "query", false, true, false);
writeParameters(gen, requiredHeaders, "header", true, false, false);
writeParameters(gen, headers, "header", false, false, false);
gen.writeEndArray();
}

Expand Down Expand Up @@ -364,14 +372,18 @@ private void writeParameters(
final Map<String, StringValueTypeDefinition<?>> fields,
final String parameterUsedIn,
final boolean isMandatoryField,
final boolean isList)
final boolean isList,
final boolean allowEmptyValue)
throws IOException {
for (Map.Entry<String, StringValueTypeDefinition<?>> entry : fields.entrySet()) {
gen.writeStartObject();
gen.writeObjectField("name", entry.getKey());
if (isMandatoryField) {
gen.writeObjectField("required", true);
}
if (allowEmptyValue) {
gen.writeObjectField("allowEmptyValue", true);
}
gen.writeObjectField("in", parameterUsedIn);
gen.writeFieldName("schema");
if (isList) { // Handle list parameter
Expand All @@ -384,6 +396,7 @@ private void writeParameters(
} else { // Handle regular parameter
entry.getValue().serializeOpenApiTypeOrReference(gen);
}

gen.writeEndObject();
}
}
Expand Down Expand Up @@ -430,6 +443,8 @@ public static class EndpointMetaDataBuilder {
private boolean requiredRequestBody = true;
private final Map<String, StringValueTypeDefinition<?>> pathParams = new LinkedHashMap<>();
private final Map<String, StringValueTypeDefinition<?>> queryParams = new LinkedHashMap<>();
private final Map<String, StringValueTypeDefinition<?>> queryParamsAllowEmpty =
new LinkedHashMap<>();
private final Map<String, StringValueTypeDefinition<?>> requiredQueryParams =
new LinkedHashMap<>();
private final Map<String, StringValueTypeDefinition<?>> queryListParams = new LinkedHashMap<>();
Expand Down Expand Up @@ -464,21 +479,27 @@ public EndpointMetaDataBuilder pathParam(final ParameterMetadata<?> parameterMet

public EndpointMetaDataBuilder queryParam(final ParameterMetadata<?> parameterMetadata) {
final String param = parameterMetadata.getName();
if (queryParams.containsKey(param)
|| requiredQueryParams.containsKey(param)
|| queryListParams.containsKey(param)) {
if (queryParamAlreadyAdded(param)) {
throw new IllegalStateException("Query parameters already contains " + param);
}
queryParams.put(parameterMetadata.getName(), parameterMetadata.getType());
return this;
}

public EndpointMetaDataBuilder queryParamAllowsEmpty(
final ParameterMetadata<?> parameterMetadata) {
Copy link
Contributor

Choose a reason for hiding this comment

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

intelliJ suggesting to move to a dedicated method this snippet:

final String param = parameterMetadata.getName();
if (queryParams.containsKey(param)
|| queryParamsAllowEmpty.containsKey(param)
|| requiredQueryParams.containsKey(param)
|| queryListParams.containsKey(param)) {
throw new IllegalStateException("Query parameters already contains " + param);
}

makes sense to me

Copy link
Contributor

Choose a reason for hiding this comment

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

oh i thought this was used somewhere - technically a list could be expressed as
validator_id=[1,2,3,4] or validator_id=1&validator_id=2&validator_id=3&validator_id=4 if you really want to make your life difficult...
eg. https://medium.com/@AADota/spring-passing-list-and-array-of-values-as-url-parameters-1ed9bbdf0cb2

anyway - TL/DR, lets not do this.

final String param = parameterMetadata.getName();
if (queryParamAlreadyAdded(param)) {
throw new IllegalStateException("Query parameters already contains " + param);
}
queryParamsAllowEmpty.put(parameterMetadata.getName(), parameterMetadata.getType());
return this;
}

public EndpointMetaDataBuilder queryParamRequired(
final ParameterMetadata<?> parameterMetadata) {
final String param = parameterMetadata.getName();
if (queryParams.containsKey(param)
|| requiredQueryParams.containsKey(param)
|| queryListParams.containsKey(param)) {
if (queryParamAlreadyAdded(param)) {
throw new IllegalStateException("Query parameters already contains " + param);
}
requiredQueryParams.put(parameterMetadata.getName(), parameterMetadata.getType());
Expand All @@ -487,15 +508,20 @@ public EndpointMetaDataBuilder queryParamRequired(

public EndpointMetaDataBuilder queryListParam(final ParameterMetadata<?> parameterMetadata) {
final String param = parameterMetadata.getName();
if (queryParams.containsKey(param)
|| requiredQueryParams.containsKey(param)
|| queryListParams.containsKey(param)) {
if (queryParamAlreadyAdded(param)) {
throw new IllegalStateException("Query parameters already contains " + param);
}
queryListParams.put(parameterMetadata.getName(), parameterMetadata.getType());
return this;
}

private boolean queryParamAlreadyAdded(final String param) {
return queryParams.containsKey(param)
|| queryParamsAllowEmpty.containsKey(param)
|| requiredQueryParams.containsKey(param)
|| queryListParams.containsKey(param);
}

public EndpointMetaDataBuilder headerRequired(final ParameterMetadata<?> headerMetadata) {
checkRequestHeader(headerMetadata);
requiredHeaders.put(headerMetadata.getName(), headerMetadata.getType());
Expand Down Expand Up @@ -752,6 +778,7 @@ public EndpointMetadata build() {
tags,
pathParams,
queryParams,
queryParamsAllowEmpty,
requiredQueryParams,
queryListParams,
requiredHeaders,
Expand Down