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

@@ -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:
+        let
+          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)
-        if request.headers.getString("broadcast_validation") != "gossip":
+              contentBody.get()
-          # 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")
         var
           restBlock = decodeBodyJsonOrSsz(RestPublishedSignedBlockContents,
                                           body, version).valueOr:

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"

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:

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:

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"}}]
+    }
   }
 ]

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