Skip to content

Persistence transaction tracking in the database #1364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 12, 2018

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Apr 11, 2018

This is some preparatory work to get to no-rescan startups. We store transactions we own, watch or broadcast in a DB table. This allows us to track their depth and recover the transactions for debugging purposes. The transactions will also be used by the onchaind replay (next PR) to feed transactions in and restore its state.

cdecker added 6 commits April 11, 2018 17:30
Currently these are either transactions we sent ourselves or transactions that
we are watching because they are part of a channel.

Signed-off-by: Christian Decker <[email protected]>
This will later allow us to determine the transaction confirmation count, and
recover transactions for rebroadcasts.

Signed-off-by: Christian Decker <[email protected]>
All of the callback functions were only using the tx to generate the txid again,
so we just pass that in directly and save passing the tx itself.

This is a simplification to move to the DB backed depth callbacks. It'd be
rather wasteful to read the rawtx and deserialize just to serialize right away
again to find the txid, when we already searched the DB for exactly that txid.

Signed-off-by: Christian Decker <[email protected]>
We are slowly hollowing out the in-memory blockchain representation to make
restarts easier.

Signed-off-by: Christian Decker <[email protected]>
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments only, feel free to address them and apply as soon as you want (I'm away next week, don't want to block progress!)

bitcoin_txid(tx, &txid);
sqlite3_bind_sha256(stmt, 1, &txid.shad.sha);
known = sqlite3_step(stmt) == SQLITE_ROW;
sqlite3_finalize(stmt);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a debugging check, in which case it'd be better to check that the other fields haven't changed? Plus add a /*FIXME: change to INSERT OR UPDATE */ for later optimization?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I get the question. I could change this to become SELECT count(*) FROM transactions WHERE id=? and then check that I get a 1 as a result, but the effect is the same, either I have a row or I don't, and depending on which it is I need to insert or update.

Or are you saying we should fetch all fields and check that they are identical?

wallet/wallet.c Outdated
if (!sqlite3_column_int(stmt, 0))
goto fail;

loc = talz(ctx, struct txlocator);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid talz, use tal.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, cut & paste, but still an antipattern.

wallet/wallet.c Outdated

}

if (!sqlite3_column_int(stmt, 0))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an explicit sqlite3_column_null ?

@cdecker cdecker force-pushed the tracktransactions branch from 4d6806d to 7482bf8 Compare April 12, 2018 21:21
cdecker added 2 commits April 12, 2018 23:24
The only use for these was to compute their txids so we could notify depth
in case of reorgs.

Signed-off-by: Christian Decker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants