Skip to content
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

Ensure there are no duplicated script_pubkeys in sqlite #806

Merged
merged 3 commits into from
Nov 30, 2022

Conversation

afilini
Copy link
Member

@afilini afilini commented Nov 26, 2022

Description

Add a UNIQUE constraint on the script_pubkeys table so that it doesn't grow constantly when caching new addresses.

Fixes #801

Notes to the reviewers

Adding it to the 0.25 milestone since it's just a bugfix.

Still in draft because I need to add extra migration queries to clean up existing dbs.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

Add a `UNIQUE` constraint on the script_pubkeys table so that it doesn't
grow constantly when caching new addresses.

Fixes bitcoindevkit#801
@notmandatory
Copy link
Member

Oops, I didn't mean to remove this from 0.25, I'll hold off on the release until this is ready.

@notmandatory
Copy link
Member

notmandatory commented Nov 27, 2022

Here's a script to remove duplicates:

ALTER TABLE script_pubkeys RENAME TO script_pubkeys_old;
DROP INDEX idx_keychain_child;
DROP INDEX idx_script;
CREATE TABLE script_pubkeys (keychain TEXT, child INTEGER, script BLOB);
CREATE INDEX idx_keychain_child ON script_pubkeys(keychain, child);
CREATE INDEX idx_script ON script_pubkeys(script);
CREATE UNIQUE INDEX idx_script_pks_unique ON script_pubkeys(keychain, child);
INSERT OR REPLACE INTO script_pubkeys SELECT keychain, child, script FROM script_pubkeys_old;
DROP TABLE script_pubkeys_old;

EDIT: I added dropping and recreating the script_pubkey indexes because I'm not sure they are updated when the table they are indexing is renamed and a new one is created.

There's also a way to do it without creating/dropping tables but I think it has some limit on the number of rowids in the NOT IN section:

DELETE FROM script_pubkeys WHERE rowid NOT IN (SELECT min(rowid) FROM script_pubkeys GROUP BY keychain,child);

@notmandatory notmandatory added the bug Something isn't working label Nov 27, 2022
@notmandatory
Copy link
Member

This should be complete now so I marked it as ready for review. I verified the new sqlite migration scripts manually against a db with inserted duplicate script_pubkeys rows.

@notmandatory notmandatory marked this pull request as ready for review November 30, 2022 00:19
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK b5fcddc

@notmandatory notmandatory merged commit 4c5ceaf into bitcoindevkit:master Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

slow sync and big script_pubkeys table (sqlite)
2 participants