-
Notifications
You must be signed in to change notification settings - Fork 25
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
Ignore non-standard outputs when searching for the witness commitment hash #373
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
Quality Gate passedIssues Measures |
Add witness related constants that were removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
public static final byte[] WITNESS_COMMITMENT_HEADER = Hex.decode("aa21a9ed"); | ||
public static final Sha256Hash WITNESS_RESERVED_VALUE = Sha256Hash.ZERO_HASH; | ||
public static final int WITNESS_COMMITMENT_LENGTH = WITNESS_COMMITMENT_HEADER.length + Sha256Hash.LENGTH; | ||
public static final int MINIMUM_WITNESS_COMMITMENT_SIZE = WITNESS_COMMITMENT_LENGTH + 2; // 1 extra by for OP_RETURN and another one for data length |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static final int MINIMUM_WITNESS_COMMITMENT_SIZE = WITNESS_COMMITMENT_LENGTH + 2; // 1 extra by for OP_RETURN and another one for data length | |
public static final int MINIMUM_WITNESS_COMMITMENT_SIZE = WITNESS_COMMITMENT_LENGTH + 2; // 1 extra byte for OP_RETURN and another one for data length |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Fix a typo in a comment.
00ae431
to
3d07157
Compare
- Updated BridgeEventLoggerImpl constructor call - Fix failing tests - Update places using BitcoinUtils.findWitnessCommitment to use new method signature.
3d07157
to
3ef94c3
Compare
Replaced by #378 |
Ignore non-standard outputs when searching for the witness commitment hash
## Motivation and Context
Coinbase transactions may sometimes contain non-standard outputs that cause the Bridge to fail parsing them. When iterating through the outputs of a coinbase transaction in search for the witness commitment hash, non-standard outputs should be ignored and continue searching through the remaining outputs.
How Has This Been Tested?
Unit tests
Types of changes
Checklist:
rskj:coinbase-parsing-integration-rebased