lightningd, pyln-testing: do extra checks to make sure check *cannot*…

… write to db.

Put an assertion inside db.c, and run every command we do (in testing) through
a `check` variant.

I inserted a deliberate bug (made addpsbtoutput call wallet_get_newindex()
before returning when running `check`, and indeed, backtrace as expected.

Signed-off-by: Rusty Russell <[email protected]>

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

@@ -715,6 +715,22 @@ def call(self, method, payload=None, cmdprefix=None, filter=None):
                     testpayload[k] = v
             schemas[0].validate(testpayload)
 
+        if method != 'check':
+            if isinstance(payload, dict):
+                checkpayload = payload.copy()
+                checkpayload['command_to_check'] = method
+            elif payload is None:
+                checkpayload = [method]
+            else:
+                checkpayload = [method] + list(payload)
+
+            # This can fail, that's fine!  But causes lightningd to check
+            # that we don't access db.
+            try:
+                LightningRpc.call(self, 'check', checkpayload)
+            except ValueError:
+                pass
+
         res = LightningRpc.call(self, method, payload, cmdprefix, filter)
         self.logger.debug(json.dumps({
             "id": id,

db/common.h

@@ -64,6 +64,9 @@ struct db {
 
 	/* Set by --developer */
 	bool developer;
+
+	/* Fatal if we try to write to db */
+	bool readonly;
 };
 
 struct db_query {

db/exec.c

@@ -142,6 +142,11 @@ bool db_in_transaction(struct db *db)
 	return db->in_transaction;
 }
 
+void db_set_readonly(struct db *db, bool readonly)
+{
+	db->readonly = readonly;
+}
+
 void db_commit_transaction(struct db *db)
 {
 	bool ok;

db/exec.h

@@ -48,5 +48,9 @@ bool db_in_transaction(struct db *db);
  */
 void db_commit_transaction(struct db *db);
 
+/**
+ * db_set_readonly - make writes fatal or allowed.
+ */
+void db_set_readonly(struct db *db, bool readonly);
 
 #endif /* LIGHTNING_DB_EXEC_H */

db/utils.c

@@ -173,6 +173,9 @@ void db_exec_prepared_v2(struct db_stmt *stmt TAKES)
 {
 	bool ret = stmt->db->config->exec_fn(stmt);
 
+	if (stmt->db->readonly)
+		assert(stmt->query->readonly);
+
 	/* If this was a write we need to bump the data_version upon commit. */
 	stmt->db->dirty = stmt->db->dirty || !stmt->query->readonly;
 
@@ -334,6 +337,7 @@ struct db *db_open_(const tal_t *ctx, const char *filename,
 	db->developer = developer;
 	db->errorfn = errorfn;
 	db->errorfn_arg = arg;
+	db->readonly = false;
 	list_head_init(&db->pending_statements);
 	if (!strstr(db->filename, "://"))
 		db_fatal(db, "Could not extract driver name from \"%s\"", db->filename);

lightningd/jsonrpc.c

@@ -1522,7 +1522,10 @@ static struct command_result *json_check(struct command *cmd,
 	json_tok_remove(&mod_params, mod_params, name_tok, 1);
 
 	cmd->mode = CMD_CHECK;
+	/* Make *sure* it doesn't try to manip db! */
+	db_set_readonly(cmd->ld->wallet->db, true);
 	res = cmd->json_cmd->dispatch(cmd, buffer, mod_params, mod_params);
+	db_set_readonly(cmd->ld->wallet->db, false);
 
 	/* CMD_CHECK always makes it "fail" parameter parsing. */
 	assert(res == &param_failed);