SQLite shouldn't depend on serde_json

[LLFourn]

If we depend on serde_json for serializing anchors we will never be able to remove it since it will always be needed to do migrations. Currently there is only one widely used anchor. We can just use some custom serialization or normalize it into columns. We can require that the anchor implement {To/Fom}Sql for sqlite support.

I am bringing this up in the hope that we can fix this before doing a v1 release. Beta users can be advised to delete their DB data and rebuild it by doing a full_scan once we make the change.

[notmandatory]

Agreed. I originally put this in to handle the generic Anchors, but now I think we only need height and hash and those can go in their own columns.

[nymius]

I would like to work on this.

[LLFourn]

@nymius that would be great. IMO the approach should be to just implement things for anchors that implement ToSql/FromSql. I think we should use a BLOB type and just big endian encode the numbers as big-endian integers in the case of confirmation height and time.

cc @evanlinjin

[nymius]

Ok, so your approach is to reduce the anchor table to (txid, anchor) (txid, block_height, block_hash, anchor) where txid is stored as TEXT and anchor is a single BLOB with all the metadata serialized as a single stream of bytes (with numbers encoded in big endian).

Correcting me here: Anchors will always have block height and hash available, and we need something to individualize different anchors for the same tx, so I should not remove them from table, in fact, the table will stay as it is.

[evanlinjin]

Hey @nymius was just talking to @LLFourn in person about this. The conclusion is to constrain support to just be for a single anchor type ConfirmationBlockTime. The anchor table will represent all fields of ConfirmationBlockTime: block_height, block_hash and confirmation_time (+ txid).

Here are the reasons:

1. No one is using rusqlite with any other anchor type, and if they are, they can do something custom anyway.
2. We want to change how we represent the "anchor" concept in the future, supporting for multiple Anchor types here will cause more problems for migration later on.

I think this change will need to be backwards compatible. In tx_graph::ChangeSet::init_sqlite_tables, introduce schema_v1 which migrates the bdk_anchors table to rm anchor column in place of confirmation_time. This migration can assume that the contained anchor data is ConfirmationBlockTime (even though it is a generic right now).

[nymius]

1. No one is using rusqlite with any other anchor type, and if they are, they can do something custom anyway.

I've just discovered on master re-base that #1736 breaks this assumption.

The idea was to add anchors without associated Txs, so I suppose it's ok if I modify the test to use ChangeSet<ConfirmationBlockTime> instead of ChangeSet<BlockId>.

[evanlinjin]

@nymius modifying the tests to use ConfirmationBlockTime is fine. I meant no one is using other anchors with rusqlite in production.