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

Fix assertion in CKey::SignCompact #15299

Merged
merged 1 commit into from Jan 31, 2019

Conversation

Projects
None yet
5 participants
@promag
Copy link
Member

commented Jan 31, 2019

Fixes #15286.

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

utACK 3617f11

@practicalswift

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

utACK 3617f11

This is why I love [[nodiscard]] :-)

@Empact

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

utACK 3617f11

@laanwj laanwj merged commit 3617f11 into bitcoin:master Jan 31, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jan 31, 2019

Merge #15299: Fix assertion in CKey::SignCompact
3617f11 Fix assertion in CKey::SignCompact (João Barbosa)

Pull request description:

  Fixes #15286.

Tree-SHA512: b39b6f26f87cf1850b13f625ab6de963937b6ecb5b6d4ac4932134f0491a6c0fa61c6d6e6980e8b1770775578dc365fdd1b6ba426bba1f7c23430f68b3a2339a
@maaku

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2019

Already merged, but FWIW this is just as redundant. secp256k1_ecdsa_recoverable_signature_serialize_compact always returns 1; the serialization function semantics are that they never fail. No check is required.

@laanwj

This comment has been minimized.

Copy link
Member

commented Jan 31, 2019

secp256k1_ecdsa_recoverable_signature_serialize_compact always returns 1; the serialization function semantics are that they never fail. No check is required.

That's true—however, as long as it has a return value, I think it's correct to check it. It might start to do error handling in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.