tapdb: retroactively insert old asset burns into burn table #1612
Conversation
Not yet, as we haven't hooked up inserting elements into the tree yet. We also haven't implemented retroactively creation of supply updates into the tree. |
| burn_id INTEGER PRIMARY KEY, | ||
|
|
||
| -- A reference to the primary key of the transfer that includes this burn. | ||
| transfer_id BIGINT NOT NULL REFERENCES asset_transfers(id), |
There was a problem hiding this comment.
So we only need to make this BIGINT for foreign key refs? Do we need to do this anywhere else? '
I think in some of my recent PRs, I don't declare foreign key refs as BIGINT.
There was a problem hiding this comment.
Perhaps we should add a new linting script for this?
There was a problem hiding this comment.
Yes, if the primary key that is referenced is INTEGER PRIMARY KEY then it will become a BIGSERIAL PRIMARY KEY on postgres and the foreign key should be BIGINT to match that (you'll mainly notice because you have to cast the ID from int32 to int64 and back in the tapdb Golang code.
I did a quick scan and the only other instance I found was mint_anchor_uni_commitments.batch_id. Perhaps this can be updated in one of the in-flight PRs?
I also started looking into sqlfluff that can do linting and also custom plugins, so we could add our own rule. Will create a PR soon.
Pull Request Test Coverage Report for Build 15845300932Details
💛 - Coveralls |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
This aligns the foreign key to be the same type as the referenced table's primary key. This was an oversight that resulted from our type replacement map we use to change the types of our primary keys to be compatible with what SQLite and Postgres support. So even though all our primary keys in the SQL files are defined as INTEGER PRIMARY KEY, they are rewritten to BIGSERIAL PRIMARY KEY in Postgres to allow auto increment, which corresponds to a BIGINT. See scripts/gen_sqlc_docker.sh for a more in-depth explanation.
We only added the burn table in the most recent release. But users might have burned assets before, which wouldn't be shown when listing burns (only with is_burn=true when listing assets). This migration retroactively inserts burned assets into the burn table that aren't there yet.
github-project-automation
bot
moved this from 👀 In review
to ✅ Done
in Taproot-Assets Project Board
We added the dedicated burn table after introducing the burn functionality. Which means, some asset burns users made before the table was added didn't show up when querying burns with
tapcli assets listburns(only when listing all assets, theis_burn: truewould show in the output).This fixes the problem by inserting any asset burns into the dedicated table.
We also fix an issue with the type of the foreign key.
@Roasbeef does this also have an impact on the burn tree? Should we also insert the burns there?