feat: Glock transaction graph (2) #298

uncomputable · 2025-12-09T04:06:55Z

This PR implements and tests all (connector) outputs that are used in the Glock transaction graph.

I have used AI for autocompletion.

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature/Enhancement (non-breaking change which adds functionality or enhances an existing one)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Refactor
New or updated tests
Dependency Update

Notes to Reviewers

The Connector and Signer traits turned out very useful for implementing and testing transaction outputs. While Connector is made for Taproot outputs, I was able to implement CpfpConnector which uses a custom script pubkey. I also implemented ContestCounterproofOutput, which is not a connector but just a normal output. We may want to rename Connector to just Output or something like that.

Checklist

I have performed a self-review of my code.
I have commented my code where necessary.
I have updated the documentation if needed.
My changes do not introduce new warnings.
I have added (where necessary) tests that prove my changes are effective or that my feature works.
New and existing tests pass with my changes.
I have disclosed my use of AI in the body of this PR.

Related Issues

Depends on #296

codecov · 2025-12-09T04:30:13Z

Codecov Report

❌ Patch coverage is 83.35234% with 146 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.78%. Comparing base (e0416ed) to head (22d5f47).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...ates/tx-graph2/src/connectors/timelocked_n_of_n.rs	50.64%	76 Missing ⚠️
crates/tx-graph2/src/connectors/claim_contest.rs	84.95%	17 Missing ⚠️
...s/tx-graph2/src/connectors/contest_counterproof.rs	87.50%	16 Missing ⚠️
crates/tx-graph2/src/connectors/cpfp.rs	77.04%	14 Missing ⚠️
crates/tx-graph2/src/transactions/claim.rs	0.00%	12 Missing ⚠️
crates/tx-graph2/src/connectors/n_of_n.rs	85.71%	7 Missing ⚠️
crates/tx-graph2/src/connectors/test_utils.rs	97.93%	3 Missing ⚠️
crates/tx-graph2/src/connectors/claim_payout.rs	99.03%	1 Missing ⚠️

@@            Coverage Diff             @@
##             main     #298      +/-   ##
==========================================
+ Coverage   52.10%   52.78%   +0.68%     
==========================================
  Files         148      155       +7     
  Lines       22063    22680     +617     
==========================================
+ Hits        11495    11971     +476     
- Misses      10568    10709     +141

Files with missing lines	Coverage Δ
crates/tx-graph2/src/connectors/mod.rs	`100.00% <100.00%> (ø)`
crates/tx-graph2/src/connectors/claim_payout.rs	`99.03% <99.03%> (ø)`
crates/tx-graph2/src/connectors/test_utils.rs	`97.93% <97.93%> (ø)`
crates/tx-graph2/src/connectors/n_of_n.rs	`85.71% <85.71%> (ø)`
crates/tx-graph2/src/transactions/claim.rs	`0.00% <0.00%> (ø)`
crates/tx-graph2/src/connectors/cpfp.rs	`77.04% <77.04%> (ø)`
...s/tx-graph2/src/connectors/contest_counterproof.rs	`87.50% <87.50%> (ø)`
crates/tx-graph2/src/connectors/claim_contest.rs	`84.95% <84.95%> (ø)`
...ates/tx-graph2/src/connectors/timelocked_n_of_n.rs	`50.64% <50.64%> (ø)`

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Rajil1213

Amazing work -- especially with generalizing the connectors and creating "one-test-to-rule-them-all". It does come at the cost of a more convoluted test design but I think the tradeoffs are acceptable. I also like the new TaprootWitness type and I'm okay removing the older type once the old tx-graph crate has been replaced.

Most of my review comments are about some non-obvious/new things and some general housekeeping stuff.

crates/tx-graph2/src/connectors/mod.rs

crates/tx-graph2/src/connectors/claim_contest_connector.rs

crates/tx-graph2/src/connectors/test_utils.rs

Rajil1213 · 2025-12-09T09:37:52Z

crates/tx-graph2/src/connectors/claim_payout_connector.rs

+            .push_opcode(opcodes::all::OP_SIZE)
+            .push_int(0x20)


Could you add a note in the constructor that the preimage must be a 32-byte value? At the same time, I don't really see the value in enforcing this since the spec does not specify the size of the preimage.

I looked at the stake chain implementation and it used 32-byte preimages. I think it's good to consistently use 32 bytes.

We don't have the hashlocked (stake) chain anymore so there is nothing to be consistent with. To be clear, I don't mind having this in. However, this should not be abstracted away in the implementation -- it should be documented.

It's documented now in the constructor. Should we update the spec?

crates/tx-graph2/src/connectors/test_utils.rs

crates/tx-graph2/src/connectors/contest_counterproof.rs

crates/tx-graph2/src/connectors/test_utils.rs

Rajil1213

I did a second pass of this PR, specifically the OP_CODESEPARATOR stuff and the test. I think we can make the spending transaction in the test more like what we expect to see in the final transaction graph. For the CPFP connector, it'd be useful to have a separate test that actually demonstrates the CPFP behavior. A test like this already exists in the older CPFP connector module which you might want to borrow.

crates/tx-graph2/src/connectors/test_utils.rs

All connector outputs implement this trait. Most of the taproot internals are automatically implemented. The TaprootWitness struct represents a generic Taproot witness. I realize that this clashes with the TaprootWitness from primitives/scripts/taproot.rs. I feel the latter is ill-named or should be removed, since the "witness" doesn't contain any signature data. I plan to make a follow-up PR that revises the struct and that updates the affected Musig2 signing machinery. Let's discuss this.

This commit moves the connector testing code to test_utils.rs. The code is generic, using the new Signer trait.

Stop using old connectors crate as a dependency.

It turns out that half of the Glock connectors follow the same pattern: - some internal key - a single tap leaf: N/N + some timelock

This commit extends the connector testing functionality to support leaf scripts that contain OP_CODESEPARATOR.

Because this output is only spent in one place of the transaction graph, it is strictly speaking not a connector. However, it is convenient to implement the Connector trait regardless.

uncomputable · 2025-12-09T21:02:17Z

Rebased onto main and addressed the easy comments. Still working on improving the test utils.

Rajil1213

Did another pass of this PR after the recent changes.

crates/tx-graph2/src/connectors/contest_counterproof.rs

These methods will be useful to make signartures for testing and for production. Splitting the normal method from the code separator one keeps the code simple. In the following commit, I will refactor the testing infrastructure of connectors.

This struct makes it easy to play transactions on Bitcoin Core. The RPC calls are abstracted away, to reduce code duplication. The coinbase outpoints are an easy way to fund transactions. The wallet automatically signs its inputs. This commit temporarily disables the code separator and cpfp tests. I will refactor these tests in the following commits.

Instead of using assert_connector_is_spendable, we use custom code to test the connector. Separately handling code separators keeps the code simple.

uncomputable · 2025-12-11T17:18:59Z

I refactored the testing infrastructure. It should be much simpler and more useful now. Connectors have a method to compute the sighash of their transaction input. Code separators are handled separately from the rest of the code.

uncomputable force-pushed the glock-tx-graph-connectors branch from ed566d2 to 5fe9111 Compare December 9, 2025 04:18

uncomputable marked this pull request as ready for review December 9, 2025 04:22

uncomputable requested review from Rajil1213, Zk2u and prajwolrg as code owners December 9, 2025 04:22

Rajil1213 requested changes Dec 9, 2025

View reviewed changes

Rajil1213 reviewed Dec 9, 2025

View reviewed changes

crates/tx-graph2/src/connectors/test_utils.rs Outdated Show resolved Hide resolved

Rajil1213 requested changes Dec 9, 2025

View reviewed changes

crates/tx-graph2/src/connectors/test_utils.rs Show resolved Hide resolved

Rajil1213 mentioned this pull request Dec 9, 2025

STR-1943: Bridge E2E test setup #297

Open

14 tasks

uncomputable added 14 commits December 9, 2025 16:01

doc: Module description

7eb73fe

feat: add prelude

5835d6b

refactor: Implement Connector for claim contest

414625a

refactor: Generalize connector test utils

f35060c

This commit moves the connector testing code to test_utils.rs. The code is generic, using the new Signer trait.

feat: claim payout connector

34a7d2f

feat: n-of-n connector

aa9a8b7

feat: CPFP connector

4f416c4

refactor: Use new CPFP connector

5231092

Stop using old connectors crate as a dependency.

feat: timelocked N/N connector

d51a9c1

It turns out that half of the Glock connectors follow the same pattern: - some internal key - a single tap leaf: N/N + some timelock

chore: Rename X-connector -> X

efa853c

feat: Support OP_CODESEPARATOR tests

bfe889e

This commit extends the connector testing functionality to support leaf scripts that contain OP_CODESEPARATOR.

feat: Add contest counterproof output

fefa47e

Because this output is only spent in one place of the transaction graph, it is strictly speaking not a connector. However, it is convenient to implement the Connector trait regardless.

fix: pr comments

aeda373

uncomputable force-pushed the glock-tx-graph-connectors branch from 5fe9111 to aeda373 Compare December 9, 2025 21:01

Rajil1213 reviewed Dec 10, 2025

View reviewed changes

crates/tx-graph2/src/connectors/contest_counterproof.rs Outdated Show resolved Hide resolved

Rajil1213 added P0 - Blocks Development required labels Dec 10, 2025

doc: address pr comments

f6e9f01

uncomputable added 5 commits December 11, 2025 11:50

doc: pr comment

c17e78f

feat: compute sighash of connectors

dc2ed1b

These methods will be useful to make signartures for testing and for production. Splitting the normal method from the code separator one keeps the code simple. In the following commit, I will refactor the testing infrastructure of connectors.

refactor: CPFP test

a810673

refactor: Counterproof test

22d5f47

Instead of using assert_connector_is_spendable, we use custom code to test the connector. Separately handling code separators keeps the code simple.

feat: Glock transaction graph (2) #298

Are you sure you want to change the base?

feat: Glock transaction graph (2) #298

Uh oh!

Conversation

uncomputable commented Dec 9, 2025

Type of Change

Notes to Reviewers

Checklist

Related Issues

Uh oh!

codecov bot commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Rajil1213 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rajil1213 Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

uncomputable Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Rajil1213 Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

uncomputable Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Rajil1213 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

uncomputable commented Dec 9, 2025

Uh oh!

Rajil1213 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

uncomputable commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Dec 9, 2025 •

edited

Loading