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

check SPV proof inner nodes not to be valid transactions #4436

Merged
merged 1 commit into from
Jun 20, 2018

Conversation

SomberNight
Copy link
Member

This attempts to implement protection against the attack recently described on the mailing list.

Before merging, I would like to measure the performance impact with a relatively large wallet on mainnet.
So far, I could only measure on testnet, but blocks are mostly empty there, so merkle proofs are short.

On testnet, I synced a wallet (A) that has ~350 txns.
Irrespective of this PR, the sync took ~10 seconds.
Without the patch, the verifier spent about 19 ms altogether in hash_merkle_root.
With the patch, the verifier spent about 66 ms altogether in hash_merkle_root.

On testnet, I synced a wallet (B) that has ~10.9k txns.
The sync took ~3.5 minutes. (with the patch, it was about 4 seconds longer, but that is within error probably; I only measured this once)
Without the patch, the verifier spent about 733 ms altogether in hash_merkle_root.
With the patch, the verifier spent about 2014 ms altogether in hash_merkle_root.

@SomberNight SomberNight added this to the 3.2 milestone Jun 14, 2018
@SomberNight
Copy link
Member Author

SomberNight commented Jun 15, 2018

I've found this address on mainnet: 1NcXyC1m3XE1s3u9rBZawt3sCZkeGEgoH9,
which has made 9733 transactions, all in the last 6 weeks.

When syncing a wallet for the above address, I benchmarked the verifier.

Without the patch, the verifier spent about 647 ms altogether in hash_merkle_root.
With the patch, the verifier spent about 1946 ms altogether in hash_merkle_root.
_raise_if_valid_tx was called 109422 times.

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.

1 participant