From 1a5bcb479eae2be19e58bf119202b6bc7dd0bb6c Mon Sep 17 00:00:00 2001 From: Eugene Kabanov Date: Wed, 1 Nov 2023 09:31:18 +0200 Subject: [PATCH] Fix `broadcast_validation` handling in `publishBlockV2` (fixes #5531) (#5547) * Address issue #5531. * Add more tests. * Add to resttest ability to check values. Fix tests. --- beacon_chain/rpc/rest_beacon_api.nim | 29 +++-- beacon_chain/rpc/rest_constants.nim | 2 + .../eth2_apis/eth2_rest_serialization.nim | 21 ++++ beacon_chain/spec/eth2_apis/rest_types.nim | 3 + ncli/resttest-rules.json | 104 ++++++++++++++++++ ncli/resttest.nim | 42 +++++-- 6 files changed, 183 insertions(+), 18 deletions(-) diff --git a/beacon_chain/rpc/rest_beacon_api.nim b/beacon_chain/rpc/rest_beacon_api.nim index 352a46da6..56b741999 100644 --- a/beacon_chain/rpc/rest_beacon_api.nim +++ b/beacon_chain/rpc/rest_beacon_api.nim @@ -906,19 +906,32 @@ proc installBeaconApiHandlers*(router: var RestRouter, node: BeaconNode) = # https://ethereum.github.io/beacon-APIs/#/Beacon/publishBlockV2 router.api(MethodPost, "/eth/v2/beacon/blocks") do ( + broadcast_validation: Option[BroadcastValidationType], contentBody: Option[ContentBody]) -> RestApiResponse: let res = block: - if contentBody.isNone(): - return RestApiResponse.jsonError(Http400, EmptyRequestBodyError) - if request.headers.getString("broadcast_validation") != "gossip": - # TODO (henridf): support 'consensus' and 'consensus_and_equivocation' - # broadcast_validation - return RestApiResponse.jsonError( - Http500, "gossip broadcast_validation only supported") let - body = contentBody.get() version = request.headers.getString("eth-consensus-version") + validation = + block: + let res = + if broadcast_validation.isNone(): + BroadcastValidationType.Gossip + else: + broadcast_validation.get().valueOr: + return RestApiResponse.jsonError(Http400, + InvalidBroadcastValidationType) + # TODO (henridf): support 'consensus' and + # 'consensus_and_equivocation' broadcast_validation types. + if res != BroadcastValidationType.Gossip: + return RestApiResponse.jsonError(Http500, + "Only `gossip` broadcast_validation option supported") + res + body = + block: + if contentBody.isNone(): + return RestApiResponse.jsonError(Http400, EmptyRequestBodyError) + contentBody.get() var restBlock = decodeBodyJsonOrSsz(RestPublishedSignedBlockContents, body, version).valueOr: diff --git a/beacon_chain/rpc/rest_constants.nim b/beacon_chain/rpc/rest_constants.nim index a57ddeb2a..49819d723 100644 --- a/beacon_chain/rpc/rest_constants.nim +++ b/beacon_chain/rpc/rest_constants.nim @@ -243,3 +243,5 @@ const "Invalid or missing timestamp value" InvalidSidecarIndexValueError* = "Invalid blob index" + InvalidBroadcastValidationType* = + "Invalid broadcast_validation type value" diff --git a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim index 5498610c4..b7d5789a3 100644 --- a/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim +++ b/beacon_chain/spec/eth2_apis/eth2_rest_serialization.nim @@ -3606,6 +3606,15 @@ proc encodeString*(value: StateIdent): RestResult[string] = of StateIdentType.Justified: ok("justified") +proc encodeString*(value: BroadcastValidationType): RestResult[string] = + case value + of BroadcastValidationType.Gossip: + ok("gossip") + of BroadcastValidationType.Consensus: + ok("consensus") + of BroadcastValidationType.ConsensusAndEquivocation: + ok("consensus_and_equivocation") + proc encodeString*(value: BlockIdent): RestResult[string] = case value.kind of BlockQueryKind.Slot: @@ -3821,6 +3830,18 @@ proc decodeString*(t: typedesc[BlockIdent], let res = ? Base10.decode(uint64, value) ok(BlockIdent(kind: BlockQueryKind.Slot, slot: Slot(res))) +proc decodeString*(t: typedesc[BroadcastValidationType], + value: string): Result[BroadcastValidationType, cstring] = + case value + of "gossip": + ok(BroadcastValidationType.Gossip) + of "consensus": + ok(BroadcastValidationType.Consensus) + of "consensus_and_equivocation": + ok(BroadcastValidationType.ConsensusAndEquivocation) + else: + err("Incorrect broadcast validation type value") + proc decodeString*(t: typedesc[ValidatorIdent], value: string): Result[ValidatorIdent, cstring] = if len(value) > 2: diff --git a/beacon_chain/spec/eth2_apis/rest_types.nim b/beacon_chain/spec/eth2_apis/rest_types.nim index 41459ceaf..1015cf55a 100644 --- a/beacon_chain/spec/eth2_apis/rest_types.nim +++ b/beacon_chain/spec/eth2_apis/rest_types.nim @@ -84,6 +84,9 @@ type StateIdentType* {.pure.} = enum Head, Genesis, Finalized, Justified + BroadcastValidationType* {.pure.} = enum + Gossip, Consensus, ConsensusAndEquivocation + StateIdent* = object case kind*: StateQueryKind of StateQueryKind.Slot: diff --git a/ncli/resttest-rules.json b/ncli/resttest-rules.json index 8b0999908..18bcfdcd1 100644 --- a/ncli/resttest-rules.json +++ b/ncli/resttest-rules.json @@ -3611,5 +3611,109 @@ }} ] } + }, + { + "topics": ["beacon", "beacon_blocks_publish_v2"], + "request": { + "method": "POST", + "headers": { + "Accept": "application/json", + "Content-Type": "application/json", + "Eth-Consensus-Version": "capella" + }, + "url": "/eth/v2/beacon/blocks", + }, + "response": { + "status": {"operator": "equals", "value": "400"}, + "headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}], + "body": [{"operator": "jstructcmpns", "value": {"code": 400, "message": "Empty request's body"}}] + } + }, + { + "topics": ["beacon", "beacon_blocks_publish_v2"], + "request": { + "method": "POST", + "headers": { + "Accept": "application/json", + "Content-Type": "application/json", + "Eth-Consensus-Version": "capella" + }, + "url": "/eth/v2/beacon/blocks?broadcast_validation=gossip", + }, + "response": { + "status": {"operator": "equals", "value": "400"}, + "headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}], + "body": [{"operator": "jstructcmpnsav", "value": {"code": 400, "message": "Empty request's body"}}] + } + }, + { + "topics": ["beacon", "beacon_blocks_publish_v2"], + "request": { + "method": "POST", + "headers": { + "Accept": "application/json", + "Content-Type": "application/json", + "Eth-Consensus-Version": "capella" + }, + "url": "/eth/v2/beacon/blocks?broadcast_validation=test", + }, + "response": { + "status": {"operator": "equals", "value": "400"}, + "headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}], + "body": [{"operator": "jstructcmpnsav", "value": {"code": 400, "message": "Invalid broadcast_validation type value"}}] + } + }, + { + "topics": ["beacon", "beacon_blocks_publish_v2"], + "request": { + "method": "POST", + "headers": { + "Accept": "application/json", + "Content-Type": "application/json", + "Eth-Consensus-Version": "capella" + }, + "url": "/eth/v2/beacon/blocks?broadcast_validation=", + }, + "response": { + "status": {"operator": "equals", "value": "400"}, + "headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}], + "body": [{"operator": "jstructcmpnsav", "value": {"code": 400, "message": "Invalid broadcast_validation type value"}}] + } + }, + { + "topics": ["beacon", "beacon_blocks_publish_v2"], + "request": { + "method": "POST", + "headers": { + "Accept": "application/json", + "Content-Type": "application/json", + "Eth-Consensus-Version": "capella" + }, + "url": "/eth/v2/beacon/blocks?broadcast_validation=consensus", + }, + "comment": "TODO: This should be replaced when `consensus` become supported", + "response": { + "status": {"operator": "equals", "value": "500"}, + "headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}], + "body": [{"operator": "jstructcmpnsav", "value": {"code": 500, "message": "Only `gossip` broadcast_validation option supported"}}] + } + }, + { + "topics": ["beacon", "beacon_blocks_publish_v2"], + "request": { + "method": "POST", + "headers": { + "Accept": "application/json", + "Content-Type": "application/json", + "Eth-Consensus-Version": "capella" + }, + "url": "/eth/v2/beacon/blocks?broadcast_validation=consensus_and_equivocation", + }, + "comment": "TODO: This should be replaced when `consensus_and_equivocation` become supported", + "response": { + "status": {"operator": "equals", "value": "500"}, + "headers": [{"key": "Content-Type", "value": "application/json", "operator": "equals"}], + "body": [{"operator": "jstructcmpnsav", "value": {"code": 500, "message": "Only `gossip` broadcast_validation option supported"}}] + } } ] diff --git a/ncli/resttest.nim b/ncli/resttest.nim index c8a9c47af..c1150d91a 100644 --- a/ncli/resttest.nim +++ b/ncli/resttest.nim @@ -34,7 +34,8 @@ type Exists, NotExists, Equals, OneOf, Substr BodyOperatorKind {.pure.} = enum - Exists, JsonStructCmpS, JsonStructCmpNS + Exists, JsonStructCmpS, JsonStructCmpNS, + JsonStructCmpSAV, JsonStructCmpNSAV StatusExpect = object kind: StatusOperatorKind @@ -649,13 +650,18 @@ proc getResponseBodyExpect(rule: JsonNode): Result[BodyExpect, cstring] = BodyOperatorKind.JsonStructCmpS of "jstructcmpns": BodyOperatorKind.JsonStructCmpNS + of "jstructcmpsav": + BodyOperatorKind.JsonStructCmpSAV + of "jstructcmpnsav": + BodyOperatorKind.JsonStructCmpNSAV else: return err("`response.body` element has incorrect operator") case operator of BodyOperatorKind.Exists: res.add(BodyItemExpect(kind: operator)) - of BodyOperatorKind.JsonStructCmpS, BodyOperatorKind.JsonStructCmpNS: + of BodyOperatorKind.JsonStructCmpS, BodyOperatorKind.JsonStructCmpNS, + BodyOperatorKind.JsonStructCmpSAV, BodyOperatorKind.JsonStructCmpNSAV: let start = block: var default: seq[string] @@ -762,7 +768,7 @@ proc getPath(jobj: JsonNode, path: seq[string]): Result[JsonNode, cstring] = jnode = jitem ok(jnode) -proc structCmp(j1, j2: JsonNode, strict: bool): bool = +proc structCmp(j1, j2: JsonNode, strict: bool, checkvalue: bool): bool = if j1.kind != j2.kind: return false case j1.kind @@ -776,7 +782,7 @@ proc structCmp(j1, j2: JsonNode, strict: bool): bool = false else: for item in j1.elems: - if not(structCmp(item, j2.elems[0], strict)): + if not(structCmp(item, j2.elems[0], strict, checkvalue)): return false true of JObject: @@ -787,7 +793,7 @@ proc structCmp(j1, j2: JsonNode, strict: bool): bool = let j2node = j2.getOrDefault(key) if isNil(j2node): return false - if not(structCmp(value, j2node, strict)): + if not(structCmp(value, j2node, strict, checkvalue)): return false true else: @@ -795,10 +801,18 @@ proc structCmp(j1, j2: JsonNode, strict: bool): bool = let j1node = j1.getOrDefault(key) if isNil(j1node): return false - if not(structCmp(j1node, value, strict)): + if not(structCmp(j1node, value, strict, checkvalue)): return false true - else: + of JString: + if checkvalue: j1.str == j2.str else: true + of JInt: + if checkvalue: j1.num == j2.num else: true + of JFloat: + if checkvalue: j1.fnum == j2.fnum else: true + of JBool: + if checkvalue: j1.bval == j2.bval else: true + of JNull: true proc validateBody(body: openArray[byte], expect: BodyExpect): bool = @@ -810,7 +824,8 @@ proc validateBody(body: openArray[byte], expect: BodyExpect): bool = of BodyOperatorKind.Exists: if len(body) == 0: return false - of BodyOperatorKind.JsonStructCmpS, BodyOperatorKind.JsonStructCmpNS: + of BodyOperatorKind.JsonStructCmpS, BodyOperatorKind.JsonStructCmpNS, + BodyOperatorKind.JsonStructCmpSAV, BodyOperatorKind.JsonStructCmpNSAV: let jbody = block: let jres = jsonBody(body) @@ -822,11 +837,18 @@ proc validateBody(body: openArray[byte], expect: BodyExpect): bool = return false jpathres.get() let strict = - if item.kind == BodyOperatorKind.JsonStructCmpS: + if item.kind in {BodyOperatorKind.JsonStructCmpS, + BodyOperatorKind.JsonStructCmpSAV}: true else: false - if not(structCmp(jbody, item.value, strict)): + let checkvalue = + if item.kind in {BodyOperatorKind.JsonStructCmpSAV, + BodyOperatorKind.JsonStructCmpNSAV}: + true + else: + false + if not(structCmp(jbody, item.value, strict, checkvalue)): return false true