Add coverage for operations on primitive types and introduce JSON schemas by stringintech · Pull Request #14 · stringintech/kernel-bindings-tests

stringintech · 2026-05-14T10:06:08Z

Details included in commit descriptions.
Tested pre-release v0.0.4-alpha.1 in stringintech/go-bitcoinkernel#12 (comment).

Thank you @janb84 for your earlier work on JSON schemas in #10. I worked on the structure a bit to enforce stricter test suite validation (according to my earlier #11 (comment)) and making it easier to integrate the schemas for runtime handler response validation and method spec generation.

Also change values to better titles and adapt runner to use both title and filename

stringintech

Left some notes/questions that I would appreciate the reviewers input on.

stringintech · 2026-05-14T10:44:33Z

+    {
+      "description": "Creates a second genesis block header while ignoring trailing bytes",
+      "request": {
+        "id": "block_header#5",
+        "method": "btck_block_header_create",
+        "params": {
+          "raw_block_header": "0100000000000000000000000000000000000000000000000000000000000000000000003ba3edfd7a7b12b27ac72c3e67768f617fc81bc3888a51323a9fb8aa4b1e5e4a29ab5f49ffff001d1dac2b7cdeadbeef"
+        },
+        "ref": "$trailing_header"
+      },
+      "expected_response": {
+        "result": {
+          "ref": "$trailing_header"
+        }
+      }
+    },


I included a test case for creating a header object with an input longer than 80 bytes.
In bitcoinkernel.h it is documented that header must be 80 bytes, but the implementation is less strict by allowing longer values and simply discarding trailing bytes (sth that is also allowed via the submitheader RPC).
So I think my question is: should the header file documentation be revised?

Imo conformance should only test the interface, so until the interface is changed, I don't think it makes sense to test > 80 bytes.

I think the question of whether the interface or the implementation should be changed is probably better had on bitcoin/bitcoin?

Makes sense, thanks. I dropped the related test cases.

stringintech · 2026-05-14T10:57:36Z

+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "$ref": "./shared.json#/$defs/RefResponse",
+  "description": "Contains the created script pubkey (e.g., `{\"ref\": \"$script_pubkey\"}`)"
+}


Here I have documented that btck_script_pubkey_create method response cannot return error, which is slightly different from the C API which returns nullptr (equivalent to generic errors {} in kernel-bindings-tests):

btck_ScriptPubkey* btck_script_pubkey_create(const void* script_pubkey, size_t script_pubkey_len) { if (script_pubkey == nullptr && script_pubkey_len != 0) { return nullptr; } auto data = std::span{reinterpret_cast<const uint8_t*>(script_pubkey), script_pubkey_len}; return btck_ScriptPubkey::create(data.begin(), data.end()); }

Wouldn't script_pubkey == nullptr && script_pubkey_len != 0 only happen on programmer error (bindings implementation bug)? If yes, wouldn't having an assertion be a better fit here instead of returning nullptr?

An assertion seems sensible, yes.

Opened bitcoin/bitcoin#35312 to address this.

stringintech · 2026-05-14T11:06:03Z

+    {
+      "description": "Verifies block hash serialization round-trips to the original bytes",
+      "request": {
+        "id": "block_hash#2",
+        "method": "btck_block_hash_to_bytes",
+        "params": {
+          "block_hash": {
+            "ref": "$hash1"
+          }
+        }
+      },
+      "expected_response": {
+        "result": "6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000"
+      }
+    },


Right now btck_block_hash_to_bytes (and btck_txid_to_bytes) do not return the hex in Bitcoin Core display order (reverse). I chose this approach so the returned hex value is equivalent to the one we use in btck_block_hash_create.

stickies-v

Concept ACK. Having coverage for operations on primitive types significantly reduces the amount of unit tests each binding needs to implement, when they use this project.

Didn't review the json spec in too much detail, but format seems sensible. Always nice to have things more structured, just slightly worried too much verbosity slows down development velocity, but i suppose with AI tooling that's not really a concern anymore.

Updated stickies-v/py-bitcoinkernel#42 without significant issues (except the >80 bytes header one, which I think should be skipped).

stickies-v · 2026-05-14T13:36:52Z

This file is autogenerated from the json specs, right? Probably good to document that up top so people don't try to PR changes to this file, and to add a CI job to check that methods-spec.md is in sync with th spec?

Yes it is. CI check was already added; updated the README to include your suggestion 👍

I think it's helpful when the autogenerated file itself has a docstring at the top to indicate that "this file is autogenerated, don't edit it manually".

Done. Thanks!

stickies-v · 2026-05-15T13:25:59Z

+    {
+      "description": "Creates a second genesis block header while ignoring trailing bytes",
+      "request": {
+        "id": "block_header#5",
+        "method": "btck_block_header_create",
+        "params": {
+          "raw_block_header": "0100000000000000000000000000000000000000000000000000000000000000000000003ba3edfd7a7b12b27ac72c3e67768f617fc81bc3888a51323a9fb8aa4b1e5e4a29ab5f49ffff001d1dac2b7cdeadbeef"
+        },
+        "ref": "$trailing_header"
+      },
+      "expected_response": {
+        "result": {
+          "ref": "$trailing_header"
+        }
+      }
+    },


Imo conformance should only test the interface, so until the interface is changed, I don't think it makes sense to test > 80 bytes.

I think the question of whether the interface or the implementation should be changed is probably better had on bitcoin/bitcoin?

stickies-v · 2026-05-15T13:27:38Z

+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "$ref": "./shared.json#/$defs/RefResponse",
+  "description": "Contains the created script pubkey (e.g., `{\"ref\": \"$script_pubkey\"}`)"
+}


An assertion seems sensible, yes.

New suites added for operations on block, block hash, block header, script pubkey, transaction, transaction input, transaction output and txid. Chain suite refactored to return ref for `btck_block_tree_entry_get_block_hash` test cases and call `btck_block_hash_to_bytes` for asserting its value. Existing suites refactored to call destroy as soon as objects (refs) no longer needed and use `"expected_response": {}` where both `result` and `error` are expected to be null.

Adds suite-schema.json which is used to validate each test suite in testdata dir against. Adds a separate schema file per method which is referenced by the root schema file suite-schema.json. Response schema files are also introduced separately (and referenced by their parent method schema file). This allows the runner to later only embed the response schemas for handler response validation. A validator command line tool is written in cmd/suite-validate which performs the testdata suites validation against the root schema and depends on a third party library (github.com/santhosh-tekuri/jsonschema/v6). Co-authored-by: janb84 <githubjanb.drainer976@passmail.net>

Now the runner binary embeds the response schema files and uses the corresponding method response schema for a more strict validation of an incoming response from handler.

stringintech · 2026-05-16T14:59:44Z

Thank you for the review @stickies-v!
New pre-release which addresses your comments and #15 can be tested using v0.0.4-alpha.2 (compare).

stickies-v · 2026-05-18T10:57:14Z

LGTM. I was on the fence about the JSON schemas initially, but I think the fact that it better allows ensuring that the documentation matches the tests you ship is pretty cool, and worth the complexity.

Didn't review in detail, but tests are green now in stickies-v/py-bitcoinkernel#42 without having to make any awkward changes.

Adds a generator in cmd/specgen that deterministically generates a markdown file documenting methods based on method schema files. Now handle-spec points to the generated document using the generator.

stringintech · 2026-05-18T13:32:57Z

Force pushed to address #14 (comment) and drop the last commit (cf0e26f) in order to address #15 in a separate PR. Tested pre-release v0.0.4-alpha.3 in stringintech/go-bitcoinkernel#12 (comment) - shouldn't need testing if you already tested with alpha.2.

Rename "name" to "title" in suites

ccb99bd

Also change values to better titles and adapt runner to use both title and filename

stringintech commented May 14, 2026

View reviewed changes

stickies-v mentioned this pull request May 14, 2026

Test verbosity in output #15

Open

stickies-v reviewed May 15, 2026

View reviewed changes

stringintech and others added 3 commits May 16, 2026 17:48

Validate handler response against JSON schema

870190e

Now the runner binary embeds the response schema files and uses the corresponding method response schema for a more strict validation of an incoming response from handler.

stringintech force-pushed the jsonschema branch 3 times, most recently from b8427ae to cf0e26f Compare May 16, 2026 14:47

stringintech linked an issue May 16, 2026 that may be closed by this pull request

Test verbosity in output #15

Open

stringintech mentioned this pull request May 18, 2026

kernel: assert invalid buffer preconditions in btck_*_create functions bitcoin/bitcoin#35312

Open

stringintech removed a link to an issue May 18, 2026

Test verbosity in output #15

Open

stringintech added 3 commits May 18, 2026 16:38

Add method spec generator

b443342

Adds a generator in cmd/specgen that deterministically generates a markdown file documenting methods based on method schema files. Now handle-spec points to the generated document using the generator.

Update CI to validate suites and generated methods spec

9c1b9e0

Update README for runner releases and schema tooling

fdffbb9

stringintech force-pushed the jsonschema branch from cf0e26f to fdffbb9 Compare May 18, 2026 13:31

Conversation

stringintech commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stringintech left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stickies-v left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stringintech commented May 16, 2026

Uh oh!

stickies-v commented May 18, 2026

Uh oh!

stringintech commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

stringintech commented May 14, 2026 •

edited

Loading

stringintech commented May 18, 2026 •

edited

Loading