pyln-testing: check the request schemas.

This means suppressing schemas in some places too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

contrib/pyln-testing/pyln/testing/fixtures.py

@@ -206,7 +206,7 @@ def throttler(test_base_dir):
     yield Throttler(test_base_dir)
 
 
-def _extra_validator():
+def _extra_validator(is_request: bool):
     """JSON Schema validator with additions for our specialized types"""
     def is_hex(checker, instance):
         """Hex string"""
@@ -340,7 +340,15 @@ def _extra_validator():
             return False
         return True
 
-    def is_msat(checker, instance):
+    def is_msat_request(checker, instance):
+        """msat fields can be raw integers, sats, btc."""
+        try:
+            Millisatoshi(instance)
+            return True
+        except TypeError:
+            return False
+
+    def is_msat_response(checker, instance):
         """String number ending in msat"""
         return type(instance) is Millisatoshi
 
@@ -374,6 +382,11 @@ def _extra_validator():
             return True
         return is_msat_request(checker, instance)
 
+    # "msat" for request can be many forms
+    if is_request:
+        is_msat = is_msat_request
+    else:
+        is_msat = is_msat_response
     type_checker = jsonschema.Draft7Validator.TYPE_CHECKER.redefine_many({
         "hex": is_hex,
         "u64": is_u64,
@@ -399,15 +412,15 @@ def _extra_validator():
                                         type_checker=type_checker)
 
 
-def _load_schema(filename):
+def _load_schema(filename, is_request):
     """Load the schema from @filename and create a validator for it"""
     with open(filename, 'r') as f:
-        return _extra_validator()(json.load(f))
+        return _extra_validator(is_request)(json.load(f))
 
 
 @pytest.fixture(autouse=True)
 def jsonschemas():
-    """Load schema files if they exist"""
+    """Load schema files if they exist: returns request/response schemas by pairs"""
     try:
         schemafiles = os.listdir('doc/schemas')
     except FileNotFoundError:
@@ -415,10 +428,20 @@ def jsonschemas():
 
     schemas = {}
     for fname in schemafiles:
-        if not fname.endswith('.schema.json'):
+        if fname.endswith('.schema.json'):
+            base = fname.rpartition('.schema')[0]
+            is_request = False
+            index = 1
+        elif fname.endswith('.request.json'):
+            base = fname.rpartition('.request')[0]
+            is_request = True
+            index = 0
+        else:
             continue
-        schemas[fname.rpartition('.schema')[0]] = _load_schema(os.path.join('doc/schemas',
+        if base not in schemas:
-                                                                            fname))
+            schemas[base] = [None, None]
+        schemas[base][index] = _load_schema(os.path.join('doc/schemas', fname),
+                                            is_request)
     return schemas
 
 

contrib/pyln-testing/pyln/testing/utils.py

@@ -628,22 +628,36 @@ class PrettyPrintingLightningRpc(LightningRpc):
             patch_json,
         )
         self.jsonschemas = jsonschemas
+        self.check_request_schemas = True
 
     def call(self, method, payload=None):
         id = self.next_id
+        schemas = self.jsonschemas.get(method)
         self.logger.debug(json.dumps({
             "id": id,
             "method": method,
             "params": payload
         }, indent=2))
+
+        # We only check payloads which are dicts, which is what we
+        # usually use: there are some cases which tests [] params,
+        # which we ignore.
+        if schemas and schemas[0] and isinstance(payload, dict) and self.check_request_schemas:
+            # fields which are None are explicitly removed, so do that now
+            testpayload = {}
+            for k, v in payload.items():
+                if v is not None:
+                    testpayload[k] = v
+            schemas[0].validate(testpayload)
+
         res = LightningRpc.call(self, method, payload)
         self.logger.debug(json.dumps({
             "id": id,
             "result": res
         }, indent=2))
 
-        if method in self.jsonschemas:
+        if schemas and schemas[1]:
-            self.jsonschemas[method].validate(res)
+            schemas[1].validate(res)
 
         return res
 
@@ -1142,9 +1156,14 @@ class LightningNode(object):
                 maxfeepercent=None, retry_for=None,
                 maxdelay=None, exemptfee=None, use_shadow=True, exclude=[]):
         """Wrapper for rpc.dev_pay which suppresses the request schema"""
-        return self.rpc.dev_pay(bolt11, msatoshi, label, riskfactor,
+        # FIXME? dev options are not in schema
-                                maxfeepercent, retry_for,
+        old_check = self.rpc.check_request_schemas
-                                maxdelay, exemptfee, use_shadow, exclude)
+        self.rpc.check_request_schemas = False
+        ret = self.rpc.dev_pay(bolt11, msatoshi, label, riskfactor,
+                               maxfeepercent, retry_for,
+                               maxdelay, exemptfee, use_shadow, exclude)
+        self.rpc.check_request_schemas = old_check
+        return ret
 
     def dev_invoice(self, msatoshi, label, description, expiry=None, fallbacks=None, preimage=None, exposeprivatechannels=None, cltv=None, dev_routes=None):
         """Wrapper for rpc.invoice() with dev-routes option"""

tests/test_invoices.py

@@ -529,6 +529,7 @@ def test_waitanyinvoice(node_factory, executor):
     r = executor.submit(l2.rpc.waitanyinvoice, pay_index, 0).result(timeout=5)
     assert r['label'] == 'inv4'
 
+    l2.rpc.check_request_schemas = False
     with pytest.raises(RpcError):
         l2.rpc.waitanyinvoice('non-number')
 

tests/test_misc.py

@@ -493,6 +493,7 @@ def test_withdraw_misc(node_factory, bitcoind, chainparams):
 
     waddr = l1.bitcoin.getnewaddress()
     # Now attempt to withdraw some (making sure we collect multiple inputs)
+    l1.rpc.check_request_schemas = False
     with pytest.raises(RpcError):
         l1.rpc.withdraw('not an address', amount)
     with pytest.raises(RpcError):
@@ -501,6 +502,7 @@ def test_withdraw_misc(node_factory, bitcoind, chainparams):
         l1.rpc.withdraw(waddr, -amount)
     with pytest.raises(RpcError, match=r'Could not afford'):
         l1.rpc.withdraw(waddr, amount * 100)
+    l1.rpc.check_request_schemas = True
 
     out = l1.rpc.withdraw(waddr, amount)
 
@@ -1516,6 +1518,7 @@ def test_configfile_before_chdir(node_factory):
 def test_json_error(node_factory):
     """Must return valid json even if it quotes our weirdness"""
     l1 = node_factory.get_node()
+    l1.rpc.check_request_schemas = False
     with pytest.raises(RpcError, match=r'id: should be a channel ID or short channel ID: invalid token'):
         l1.rpc.close({"tx": "020000000001011490f737edd2ea2175a032b58ea7cd426dfc244c339cd044792096da3349b18a0100000000ffffffff021c900300000000001600140e64868e2f752314bc82a154c8c5bf32f3691bb74da00b00000000002200205b8cd3b914cf67cdd8fa6273c930353dd36476734fbd962102c2df53b90880cd0247304402202b2e3195a35dc694bbbc58942dc9ba59cc01d71ba55c9b0ad0610ccd6a65633702201a849254453d160205accc00843efb0ad1fe0e186efa6a7cee1fb6a1d36c736a012103d745445c9362665f22e0d96e9e766f273f3260dea39c8a76bfa05dd2684ddccf00000000", "txid": "2128c10f0355354479514f4a23eaa880d94e099406d419bbb0d800143accddbb", "channel_id": "bbddcc3a1400d8b0bb19d40694094ed980a8ea234a4f5179443555030fc12820"})
 

tests/test_pay.py

@@ -586,11 +586,13 @@ def test_sendpay(node_factory):
     assert invoice_unpaid(l2, 'testpayment2')
 
     # Bad ID.
+    l1.rpc.check_request_schemas = False
     with pytest.raises(RpcError):
         rs = copy.deepcopy(routestep)
         rs['id'] = '00000000000000000000000000000000'
         l1.rpc.sendpay([rs], rhash, payment_secret=inv['payment_secret'])
     assert invoice_unpaid(l2, 'testpayment2')
+    l1.rpc.check_request_schemas = True
 
     # Bad payment_secret
     l1.rpc.sendpay([routestep], rhash, payment_secret="00" * 32)

tests/test_wallet.py

@@ -44,6 +44,9 @@ def test_withdraw(node_factory, bitcoind):
 
     waddr = l1.bitcoin.rpc.getnewaddress()
     # Now attempt to withdraw some (making sure we collect multiple inputs)
+
+    # These violate schemas!
+    l1.rpc.check_request_schemas = False
     with pytest.raises(RpcError):
         l1.rpc.withdraw('not an address', amount)
     with pytest.raises(RpcError):
@@ -52,6 +55,7 @@ def test_withdraw(node_factory, bitcoind):
         l1.rpc.withdraw(waddr, -amount)
     with pytest.raises(RpcError, match=r'Could not afford'):
         l1.rpc.withdraw(waddr, amount * 100)
+    l1.rpc.check_request_schemas = True
 
     out = l1.rpc.withdraw(waddr, 2 * amount)
 
@@ -216,6 +220,9 @@ def test_minconf_withdraw(node_factory, bitcoind):
     bitcoind.generate_block(1)
 
     wait_for(lambda: len(l1.rpc.listfunds()['outputs']) == 10)
+    # This violates the request schema!
+    l1.rpc.check_request_schemas = False
+
     with pytest.raises(RpcError):
         l1.rpc.withdraw(destination=addr, satoshi=10000, feerate='normal', minconf=9999999)
 
@@ -1373,6 +1380,9 @@ def test_repro_4258(node_factory, bitcoind):
 
     addr = bitcoind.rpc.getnewaddress()
 
+    # These violate the request schema!
+    l1.rpc.check_request_schemas = False
+
     # Missing array parentheses for outputs
     with pytest.raises(RpcError, match=r"Expected an array of outputs"):
         l1.rpc.txprepare(
@@ -1391,6 +1401,8 @@ def test_repro_4258(node_factory, bitcoind):
             utxos="{txid}:{output}".format(**out)
         )
 
+    l1.rpc.check_request_schemas = True
+
     tx = l1.rpc.txprepare(
         outputs=[{addr: "all"}],
         feerate="slow",