Skip to content

Support for Signet #239

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 14, 2021
Merged

Support for Signet #239

merged 1 commit into from
Jan 14, 2021

Conversation

dr-orlovsky
Copy link
Contributor

@dr-orlovsky dr-orlovsky commented Apr 19, 2020 •
edited
Loading

This is dependent on #238 and #240 and includes commit history from them (w/o this it would not build). Once these PRs will be merged I will rebase on master and clear the history so this PR can be reviewed (it has quite a few changes).

The only relevant commit in the history to Signet support is 1a4fc78

@dr-orlovsky dr-orlovsky mentioned this pull request Apr 19, 2020
Merged
@dr-orlovsky dr-orlovsky mentioned this pull request Apr 19, 2020
Merged
@romanz romanz self-assigned this Apr 21, 2020
@UkolovaOlga UkolovaOlga mentioned this pull request Apr 22, 2020
Closed
34 tasks
@romanz romanz self-requested a review April 24, 2020 19:12
@romanz
Copy link
Owner

romanz commented Apr 24, 2020

LGTM, many thanks for the contribution!
Will be happy to merge when rust-bitcoin 0.24 is released.

@Kixunil
Copy link
Contributor

Kixunil commented Apr 26, 2020

Would rebasing the changes make them more clear now?

@kallewoof kallewoof mentioned this pull request Apr 28, 2020
Closed
@dr-orlovsky
Copy link
Contributor Author

Rebased, now the history is clear. Waiting for bitcoin 0.24

@dr-orlovsky dr-orlovsky mentioned this pull request May 14, 2020
Merged
@dr-orlovsky dr-orlovsky force-pushed the signet branch 2 times, most recently from 42401d0 to 592bf77 Compare May 14, 2020 13:49
Kixunil
Kixunil reviewed May 14, 2020
View reviewed changes
Copy link
Contributor

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

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

Wow, looks so simple now! :) Interesting, it seems that electrs doesn't care about block magic numbers or something else to check the appropriate network.

Looks good so far.

@dr-orlovsky
Copy link
Contributor Author

Now, with the new rust-bitcoin version release I've updated dependency PR #240 and rebased onto it. However, to merge this we still have to wait for rust-bitcoin/rust-bitcoin#291 to be merged into rust-bitcoin and yet another version release after that

@dr-orlovsky dr-orlovsky marked this pull request as draft September 18, 2020 16:54
@dr-orlovsky
Copy link
Contributor Author

Docker container & image working with signet + whole signet package (bitcoind+electrs+lightningd):

@romanz
Copy link
Owner

romanz commented Nov 5, 2020 •
edited
Loading

CC: @kallewoof
Cherry-picked over the WIP next branch - fac0da4 seems to be working :)

$ ./query.sh signet tb1q0k8lrhuqxj32rehjg7g2sxnegrzhqawx8xkadd
[2020-11-05T14:07:36.815Z INFO  electrs_index::metrics] serving Prometheus metrics on 127.0.0.1:34224
[2020-11-05T14:07:36.815Z DEBUG electrs_index::db] opening DB with Options { path: "./db/signet", bulk_import: true, low_memory: false }
[2020-11-05T14:07:37.573Z DEBUG electrs_index::db] DB Config { compacted: false, format: 1 }
[2020-11-05T14:07:37.628Z DEBUG electrs_index::index] read 0 blocks (0.000 MB) from DB at 0 ms
[2020-11-05T14:07:37.628Z DEBUG electrs_index::map] added 0/0 headers at 0 ms
[2020-11-05T14:07:37.710Z DEBUG electrs_index::index] loaded 10433 block locations (10433 new) at 81 ms
[2020-11-05T14:07:37.711Z INFO  electrs_index::index] reading 10433 new blocks from 1 blk*.dat files
[2020-11-05T14:07:37.767Z DEBUG electrs_index::index] indexer #1: indexed 10433 blocks from file #0 into 24075 rows
[2020-11-05T14:07:37.774Z DEBUG electrs_index::index] writing index 24075 rows from 10433 blocks
[2020-11-05T14:07:37.791Z INFO  electrs_index::index] indexed 10433 new blocks, 24075 DB rows, took 0.163s
[2020-11-05T14:07:38.397Z INFO  electrs_index::db] starting config compaction
[2020-11-05T14:07:38.452Z INFO  electrs_index::db] starting headers compaction
[2020-11-05T14:07:38.497Z INFO  electrs_index::db] starting txid compaction
[2020-11-05T14:07:38.553Z INFO  electrs_index::db] starting index compaction
[2020-11-05T14:07:38.653Z INFO  electrs_index::db] finished full compaction
[2020-11-05T14:07:38.910Z INFO  electrs_index::db] auto-compactions enabled
[2020-11-05T14:07:38.924Z DEBUG electrs_index::map] added 10433/10433 headers at 13 ms
[2020-11-05T14:07:38.925Z INFO  electrs_index::map] verified 10433 blocks, tip=000000b443529edefcd6c7ba398a16ad037abd71a89f74ebedc5a9459f907975 @ 2020-11-05T14:00:00+00:00
[2020-11-05T14:07:38.925Z DEBUG electrs_index::index] 4e6eb8f2a2659a8529ecf88148cdd58d368690397610c04f85d59da4dcef97a6 has 1 txid rows
[2020-11-05T14:07:38.925Z DEBUG electrs_query] received: 10.00000000 @ 1 outputs (scanned 1 txs, 2 outputs)
[2020-11-05T14:07:38.925Z DEBUG electrs_query] spent:    0.00000000 @ 0 inputs (scanned 1 txs, 1 inputs)
[2020-11-05T14:07:38.925Z INFO  electrs_query] 71db759c6747a99e5e958c25aa6fad7ac12ae00c8ec3414587d9ed454c85118a:1              10.00000000 @ 10409 tb1q0k8lrhuqxj32rehjg7g2sxnegrzhqawx8xkadd
[2020-11-05T14:07:38.925Z INFO  electrs_query] total: 10.00000000 BTC

kallewoof
kallewoof reviewed Nov 6, 2020
View reviewed changes
@kallewoof
Copy link

CC: @kallewoof
Cherry-picked over the WIP next branch - fac0da4 seems to be working :)

utACK fac0da4

@Kixunil
Copy link
Contributor

Kixunil commented Nov 27, 2020

What's blocking the merge of this? Would be nice to bump bitcoin version so that secp256k1 can be made optional cleanly when my PR gets merged. :)

@romanz
Copy link
Owner

romanz commented Nov 28, 2020

It is now merged in next branch.

@Kixunil
Copy link
Contributor

Kixunil commented Nov 28, 2020

What about merging to master? Or do you plan for next to be the next release?

@dr-orlovsky
Copy link
Contributor Author

Well, this require new signet-supporting rust-bitcoin version to be released; which has not happened yet. I monitor the situation nearly daily.

@Kixunil
Copy link
Contributor

Kixunil commented Nov 28, 2020

Hmm, the constants are already there. At least that was the problem when attempting to try electrs with my patch to remove secp256k1.

@dr-orlovsky
Copy link
Contributor Author

They are in the master, but not in crates.io (pending release)

@romanz
Copy link
Owner

romanz commented Nov 28, 2020

What about merging to master? Or do you plan for next to be the next release?

I am planning to port the missing features from master to next - e.g. configuration, better error handling - and then merge it into master as part of 0.9.* release, since it's DB schema is not backwards-compatible.
I plan to support the 0.8.* branch for the near future, but I prefer to continue the development using the new code base.

@dr-orlovsky dr-orlovsky marked this pull request as ready for review January 14, 2021 18:35
@dr-orlovsky
Copy link
Contributor Author

With rust-bitcoin 0.26 release this is finally ready to be merged into master

romanz
romanz approved these changes Jan 14, 2021
View reviewed changes
Copy link
Owner

@romanz romanz left a comment

Choose a reason for hiding this comment

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

Many thanks!

@romanz romanz merged commit 2a6881a into romanz:master Jan 14, 2021
@dr-orlovsky
Copy link
Contributor Author

@romanz can I pls ask you to publish a release (tag+crate), so I can update my docker images?

@romanz
Copy link
Owner

romanz commented Jan 15, 2021

Tagged and released :)

@kristapsk kristapsk mentioned this pull request Apr 1, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@romanz romanz romanz approved these changes

+2 more reviewers

@kallewoof kallewoof kallewoof left review comments

@Kixunil Kixunil Kixunil left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@romanz romanz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dr-orlovsky @romanz @Kixunil @kallewoof