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

Add tests for Bitcoin merkle root malleability behavior #1392

Closed
Tracked by #745
hdevalence opened this issue Nov 25, 2020 · 1 comment
Closed
Tracked by #745

Add tests for Bitcoin merkle root malleability behavior #1392

hdevalence opened this issue Nov 25, 2020 · 1 comment
Labels
A-consensus Area: Consensus rule updates C-enhancement Category: This is an improvement C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data

Comments

@hdevalence
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Bitcoin's transaction Merkle trees are badly designed, and suffer from malleability. In #1386 we address this by:

  • in the checkpoint verifier, explicitly checking that there are no duplicate transaction hashes;
  • in the block verifier, the double-spend prevention blocks blocks with duplicate transactions from being accepted.

That PR includes unit tests for the Merkle root computation itself, but it does not have integration tests for the verifiers.

Describe the solution you'd like

  • For the checkpointer: check that blocks with duplicate transactions are rejected, both before and after submission of the canonical block whose hash they collide with;

  • For the block verifier: take a block which otherwise passes all validation criteria, and duplicate transactions in such a way as to create a colliding block hash. Then, check that this otherwise-valid block is rejected.

@hdevalence hdevalence added A-consensus Area: Consensus rule updates C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Nov 25, 2020
@mpguerra mpguerra added P-Low and removed S-needs-triage Status: A bug report needs triage labels Feb 4, 2021
@dconnolly dconnolly self-assigned this Feb 4, 2021
@teor2345 teor2345 added C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data labels Feb 22, 2022
@teor2345
Copy link
Contributor

This is a test we should do for security, but we're unlikely to do it any time soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates C-enhancement Category: This is an improvement C-security Category: Security issues I-invalid-data Zebra relies on invalid or untrusted data, or sends invalid data
Projects
None yet
Development

No branches or pull requests

4 participants