Skip to content

Allow skipping merkle proofs' during subscription - #610

Merged
romanz merged 1 commit into
masterfrom
skip-merkle
Nov 7, 2021
Merged

Allow skipping merkle proofs' during subscription#610
romanz merged 1 commit into
masterfrom
skip-merkle

Conversation

@romanz

@romanz romanz commented Nov 7, 2021

Copy link
Copy Markdown
Owner

We still support them (by computing them on-demand).

3ce23ae#r59562642

@romanz

romanz commented Nov 7, 2021

Copy link
Copy Markdown
Owner Author

Tested using all12_subs.txt (warm cache).

# Without --skip-merkle-proofs-on-subscription:
$ curl -s localhost:4224 | grep 'rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"}'
rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"} 8.590418878

# With --skip-merkle-proofs-on-subscription:
$ curl -s localhost:4224 | grep 'rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"}'
rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"} 7.838893273

@romanz

romanz commented Nov 7, 2021

Copy link
Copy Markdown
Owner Author

The last commit results in a significant performance gain:

# Without --skip-merkle-proofs-on-subscription:
$ curl -s localhost:4224 | grep 'rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"}'
rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"} 8.89051942

# With --skip-merkle-proofs-on-subscription:
$ curl -s localhost:4224 | grep 'rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"}'
rpc_duration_sum{method="blockchain.scripthash.subscribe:multi"} 4.316420194

@romanz

romanz commented Nov 7, 2021

Copy link
Copy Markdown
Owner Author

Can be relevant for #539 :)
CC: @Transisto

@romanz

romanz commented Nov 7, 2021

Copy link
Copy Markdown
Owner Author

The cool thing is that merkle proofs are being client-side cached (at least in Electrum and Sparrow), so there actually no need to cache them in electrs :)

@romanz

romanz commented Nov 7, 2021

Copy link
Copy Markdown
Owner Author

It seems that we get now comparable performance to the 0.8.x branch: #539 (comment) 👀

@Kixunil Kixunil left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That's some great news! The code looks good too.

Comment thread src/status.rs
We still support them (by computing them on-demand) and caching them with `--cache-merkle-proofs`.

Also, don't compute block's txids if we don't cache merkle proofs during subscription

3ce23ae#r59562642
@romanz

romanz commented Nov 7, 2021

Copy link
Copy Markdown
Owner Author

The cool thing is that merkle proofs are being client-side cached (at least in Electrum and Sparrow), so there actually no need to cache them in electrs :)

I've flipped the logic so we don't cache merkle proofs by default :)

@romanz
romanz merged commit 2d09854 into master Nov 7, 2021
@romanz
romanz deleted the skip-merkle branch November 7, 2021 19:50
@Kixunil

Kixunil commented Nov 8, 2021

Copy link
Copy Markdown
Contributor

I wonder is there actually any reason to enable caching? Maybe having the option is not actually useful?

romanz added a commit that referenced this pull request Nov 8, 2021
@romanz

romanz commented Nov 8, 2021

Copy link
Copy Markdown
Owner Author

Maybe having the option is not actually useful?

image

crypto-lancern8km9 added a commit to crypto-lancern8km9/metashrew that referenced this pull request Nov 14, 2025
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