tapdb+universe: implement initial version of universe ignore tree by Roasbeef · Pull Request #1420 · lightninglabs/taproot-assets

Roasbeef · 2025-03-05T05:05:22Z

In this PR, we implement the initial version of the universe Ignore Tree.

Along the way, we need a migration to the universe leaves, as the current unique index structure prevents us from adding two leaves that have the same script key, but distinct namespace (eg: transfer to a script key, then a burn of that).

With that in place, we're able to implement the ignore tree using the existing MS-SMT/Universe routines we already have in the database.

TODO

Consistent interfaces.
Unit tests for the database layer

coveralls · 2025-03-07T01:30:24Z

Pull Request Test Coverage Report for Build 14483915227

Details

316 of 419 (75.42%) changed or added relevant lines in 6 files are covered.
10 unchanged lines in 4 files lost coverage.
Overall coverage increased (+0.2%) to 28.6%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
cmd/merge-sql-schemas/main.go	0	2	0.0%
asset/asset.go	0	7	0.0%
universe/interface.go	0	8	0.0%
universe/ignore_records.go	81	121	66.94%
tapdb/ignore_tree.go	229	275	83.27%

Files with Coverage Reduction	New Missed Lines	%
asset/group_key.go	2	57.89%
tapgarden/caretaker.go	2	68.63%
tappsbt/create.go	2	26.74%
commitment/tap.go	4	71.59%

Totals
Change from base Build 14474635473:	0.2%
Covered Lines:	26254
Relevant Lines:	91798

💛 - Coveralls

Roasbeef · 2025-03-09T02:20:44Z

Removed from draft.

Roasbeef · 2025-03-21T00:52:12Z

Pushed up a new version (CI was failing as we had two migration 30's). PTAL.

lightninglabs-deploy · 2025-03-28T01:50:07Z

@GeorgeTsagk: review reminder

Roasbeef · 2025-04-02T23:19:02Z

Will fix the collision issues in the burn tree PR.

This is ready for review (no changes since my last push to fix CI after the rebase).

guggero

Nice!
Most of my comments are around the SQL migration, the rest looks great.
Love all the rapid tests 💯

guggero · 2025-04-04T11:56:38Z

+    timestamp TIMESTAMP NOT NULL,
+    attempt_counter BIGINT NOT NULL DEFAULT 0,
+    sync_direction TEXT NOT NULL CHECK(sync_direction IN ('push', 'pull')),
+    proof_leaf_id BIGINT NOT NULL, -- FK constraint intentionally omitted for now.


Instead of making yet another copy at the end, we could re-add the foreign key constraint to this table by using ALTER TABLE ADD FOREIGN KEY. That way we'd have a named constraint that we can just disable in the future.
The only downside is that it's not directly apparent on the table definition anymore...
And this table shouldn't be super large, so perhaps easier to just create it a couple of times, so just a thought.

IIUC with sqlite, we can't do ALTER TABLE ADD FOREIGN KEY. We can only: rename a table/column, add/drop a table/column.

Need to revisit this, but the reason I needed to make a version w/o the constraint was that the migration fails otherwise either due to the rename of one of the tables above, or because we didn't update all the refs to point to the new version.

Ah, yeah, SQLite doesn't support ALTER TABLE ... ADD CONSTRAINT: https://sqlite.org/omitted.html

guggero · 2025-04-04T11:58:21Z

 , event_timestamp BIGINT NOT NULL DEFAULT 0);

-CREATE TABLE universe_leaves (
+CREATE TABLE "universe_leaves" (


hmm, is this up-to-date? I wonder where the double quotes come from...

Yeah not sure why that was added. The way the tool works, it just reads the schema directly from the meta table in sqlite itself...

Perhaps there was some formatting or extra white space in the migration that resulted in this? Will fiddle with it a bit.

ffranr

I feel as if "tuple" is used as a a bit of a filler word here. We haven't used the word "coin" much, but this might be the right time to use it.

IgnoreCoin, AddCoins(...),

// AddCoin adds a new coin to the ignore tree.

I think the programming rule of thumb is "Name the thing, not the container.", basically encapsulation which makes it cheaper to change the underlying data type if we ever need to.

Roasbeef · 2025-04-10T01:53:36Z

I feel as if "tuple" is used as a a bit of a filler word here.

Tuple as in: https://en.wikipedia.org/wiki/Tuple.

It's not really a "coin" in the context we usually use that term within Bitcoin.

ffranr · 2025-04-10T16:12:43Z

I feel as if "tuple" is used as a a bit of a filler word here.

Tuple as in: https://en.wikipedia.org/wiki/Tuple.

It's not really a "coin" in the context we usually use that term within Bitcoin.

I think a term like “asset coin” could be useful. Or something similar, I'm not sure what the convention is around "coin". Basically a higher level term for an asset leaf in a Taproot Asset MS-SMT—something that conveys spendability.

ffranr · 2025-04-10T16:44:00Z

+// TupleQueryResp is the response to a query for ignore tuples.
+type TupleQueryResp = lfn.Result[lfn.Option[[]AuthenticatedIgnoreTuple]]


Why include lfn.Option here? Wouldn't an empty slice signal the same thing as a None?

I think a similar sort of question can be asked for SumQueryResp below. I assume that if there are any leaves in the tree then the sum is always > 0, am I wrong? In which case None and 0 mean the same thing.

I went back and forth a bit here: the None value makes it explicit when the query has no response value, while the empty slice is an implicit indicator.

I assume that if there are any leaves in the tree then the sum is always > 0, am I wrong? In which case None and 0 mean the same thing.

Same thing here re implicit vs explicit. If there's no tree, then a sum value isn't defined. An implicit interpretation is that if there's no tree, then the sum is zero.

I tend to prefer the explicit route.

In this commit, we fix a bug in the existing final schema generation script to ensure it includes named indexes. From the sqlite docs: > The sqlite_schema.type column will be one of the following text strings: 'table', 'index', 'view', or 'trigger' according to the type of object defined. The 'table' string is used for both ordinary and virtual tables.

… table This commit introduces a new migration (Migration 30) that modifies the unique index on the `universe_leaves` table. The unique constraint is updated from two columns (`minting_point`, `script_key_bytes`) to three columns (`minting_point`, `script_key_bytes`, `leaf_node_namespace`). Additionally, a new test case, `TestMigration30`, is added to verify the behavior of the database before and after the migration. The test ensures that inserting a duplicate row with the same `minting_point` and `script_key_bytes` but a different `leaf_node_namespace` fails before the migration and succeeds after the migration is applied. Furthermore, the `universe_leaves` table schema is updated in the generated SQL schema file to reflect the new unique constraint. Other minor changes include the addition of new proof types in the `universe` package and the introduction of new types related to ignore tuples, enhancing the overall functionality of the codebase.

guggero

LGTM 🎉

guggero · 2025-04-16T06:57:03Z

+    timestamp TIMESTAMP NOT NULL,
+    attempt_counter BIGINT NOT NULL DEFAULT 0,
+    sync_direction TEXT NOT NULL CHECK(sync_direction IN ('push', 'pull')),
+    proof_leaf_id BIGINT NOT NULL, -- FK constraint intentionally omitted for now.


Ah, yeah, SQLite doesn't support ALTER TABLE ... ADD CONSTRAINT: https://sqlite.org/omitted.html

Roasbeef force-pushed the ignore-tree-impl branch from 9db6b54 to 9d067df Compare March 6, 2025 03:09

levmi assigned Roasbeef Mar 6, 2025

levmi added this to Taproot-Assets Project Board Mar 6, 2025

github-project-automation Bot moved this to 🆕 New in Taproot-Assets Project Board Mar 6, 2025

levmi moved this from 🆕 New to 🏗 In progress in Taproot-Assets Project Board Mar 6, 2025

Roasbeef force-pushed the ignore-tree-impl branch from 2f50f47 to 11b67f8 Compare March 9, 2025 02:20

Roasbeef marked this pull request as ready for review March 9, 2025 02:20

Roasbeef requested review from a team, GeorgeTsagk and ffranr and removed request for a team March 9, 2025 02:20

Roasbeef removed the request for review from ffranr March 18, 2025 19:02

Roasbeef force-pushed the ignore-tree-impl branch from 11b67f8 to 4a29c11 Compare March 21, 2025 00:50

Roasbeef mentioned this pull request Mar 22, 2025

[feature]: create+maintain issuer burn tree #1450

Closed

Roasbeef requested a review from guggero April 2, 2025 23:18

guggero reviewed Apr 4, 2025

View reviewed changes

levmi requested review from ffranr and removed request for GeorgeTsagk April 7, 2025 15:43

ffranr reviewed Apr 8, 2025

View reviewed changes

Comment thread universe/ignore_records.go

Comment thread tapdb/ignore_tree.go

Roasbeef force-pushed the ignore-tree-impl branch from 4a29c11 to 5219a17 Compare April 10, 2025 02:43

ffranr reviewed Apr 10, 2025

View reviewed changes

Roasbeef force-pushed the ignore-tree-impl branch 2 times, most recently from f04a2a8 to ee41b84 Compare April 16, 2025 03:14

Roasbeef requested review from ffranr and guggero April 16, 2025 03:14

Roasbeef added 4 commits April 15, 2025 20:15

universe: add encode/decode routines for new SignedIgnoreTuple

9cb1e09

tapdb: add initial implementation of universe.IgnoreTree

9668caa

Roasbeef force-pushed the ignore-tree-impl branch from ee41b84 to 9668caa Compare April 16, 2025 03:15

guggero approved these changes Apr 16, 2025

View reviewed changes

ffranr approved these changes Apr 16, 2025

View reviewed changes

github-project-automation Bot moved this from 🏗 In progress to 👀 In review in Taproot-Assets Project Board Apr 16, 2025

ffranr added this pull request to the merge queue Apr 16, 2025

Merged via the queue into lightninglabs:main with commit 2da076d Apr 16, 2025
18 checks passed

github-project-automation Bot moved this from 👀 In review to ✅ Done in Taproot-Assets Project Board Apr 16, 2025

		// TupleQueryResp is the response to a query for ignore tuples.
		type TupleQueryResp = lfn.Result[lfn.Option[[]AuthenticatedIgnoreTuple]]

Conversation

Roasbeef commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 14483915227

Details

💛 - Coveralls

Uh oh!

Roasbeef commented Mar 9, 2025

Uh oh!

Roasbeef commented Mar 21, 2025

Uh oh!

lightninglabs-deploy commented Mar 28, 2025

Uh oh!

Roasbeef commented Apr 2, 2025

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guggero Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

Roasbeef Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

guggero Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

guggero Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

Roasbeef Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ffranr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Roasbeef commented Apr 10, 2025

Uh oh!

ffranr commented Apr 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ffranr Apr 10, 2025

Choose a reason for hiding this comment

Uh oh!

Roasbeef Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

guggero left a comment

Choose a reason for hiding this comment

Uh oh!

guggero Apr 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Roasbeef commented Mar 5, 2025 •

edited

Loading

coveralls commented Mar 7, 2025 •

edited

Loading

ffranr left a comment •

edited

Loading

Roasbeef Apr 10, 2025 •

edited

Loading