Skip to content

Adds importdescriptors #248

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 1 commit into from
Dec 30, 2022
Merged

Conversation

sr-gi
Copy link
Contributor

@sr-gi sr-gi commented Sep 24, 2022

Adds importdescriptors rpc. I've renamed ImportMultiRescanSince to Timestamp given it is not being used solely for multi rescan anymore.

Feels like ImportMultiResult should also be renamed, given it is now used by both importmulti and importdescriptors. I haven't found a proper name for it though.

@casey
Copy link
Contributor

casey commented Dec 29, 2022

Finally tested this! Sorry for the delay, we had to merge it into our own branch for testing. This worked for us.

@apoelstra
Copy link
Member

I'm not sure why CI isn't running -- could you make sure that cargo +nightly fmt --check runs cleanly on this PR?

@apoelstra
Copy link
Member

cc @sr-gi fixing the format is blocking merge. utACK otherwise.

Adds `importdescriptors` rpc. `ImportMultiRescanSince` to Timestamp given it is not
being used solely for MultiRescan anymore.

Feels like `ImportMultiResult` should also be renamed, given it is now used by both `importmulti`
and `importdescriptors`. I haven't found a proper name for it though.
@sr-gi sr-gi force-pushed the import_descriptors branch from 4c35184 to 718ba56 Compare December 30, 2022 14:49
@sr-gi
Copy link
Contributor Author

sr-gi commented Dec 30, 2022

cc @sr-gi fixing the format is blocking merge. utACK otherwise.

Should be fixed now. I've also rebased master given this was forking from an old tip.

@casey
Copy link
Contributor

casey commented Dec 30, 2022

Also, the commit I tested was 4c35184.

@apoelstra
Copy link
Member

Confirmed that the rebase is a pure rebase except to drop a newline that cargo fmt didn't like.

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 718ba56

@apoelstra apoelstra merged commit 6cd787a into rust-bitcoin:master Dec 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants