Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Problem: cryptographic library code diverged from "planned" upstream (fixes #1143) #1162

Merged
merged 1 commit into from
Mar 3, 2020

Conversation

tomtau
Copy link
Contributor

@tomtau tomtau commented Mar 2, 2020

Solution:

  • cherry-picked changes from the planned upstream changes to the temporary custom secp256k1 fork
  • adapted the temporary Rust library fork to it
  • changed chain-core witness structure to use the BIP-340 encoding of public keys
  • adapted client and tests to it

NOTE:
the schnorrsig proposal was stabilized as https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
the code wasn't yet merged to upstream, so it's possible that underlying cryptographic API
may change (and the corresponding Rust), but the transaction (binary/serialized) payloads
will be the same.

@tomtau
Copy link
Contributor Author

tomtau commented Mar 2, 2020

bors try

bors bot added a commit that referenced this pull request Mar 2, 2020
@bors
Copy link
Contributor

bors bot commented Mar 2, 2020

try

Build failed

@codecov
Copy link

codecov bot commented Mar 2, 2020

Codecov Report

Merging #1162 into master will increase coverage by 0.04%.
The diff coverage is 92.59%.

@@            Coverage Diff             @@
##           master    #1162      +/-   ##
==========================================
+ Coverage   63.78%   63.83%   +0.04%     
==========================================
  Files         149      149              
  Lines       19902    19874      -28     
==========================================
- Hits        12695    12686       -9     
+ Misses       7207     7188      -19
Impacted Files Coverage Δ
client-core/src/service/hd_key_service.rs 82.53% <ø> (ø) ⬆️
client-rpc/src/rpc/wallet_rpc.rs 65.09% <ø> (ø) ⬆️
client-core/src/signer/key_pair_signer.rs 93.75% <ø> (ø) ⬆️
client-core/src/wallet.rs 100% <ø> (ø) ⬆️
client-core/src/wallet/default_wallet_client.rs 41.68% <0%> (ø) ⬆️
client-rpc/src/rpc/multisig_rpc.rs 31.73% <0%> (+0.45%) ⬆️
client-core/src/multi_sig/builder.rs 89.31% <100%> (ø) ⬆️
...ient-core/src/service/multi_sig_session_service.rs 88.31% <100%> (ø) ⬆️
client-core/src/service/root_hash_service.rs 88.54% <100%> (-0.12%) ⬇️
chain-core/src/tx/witness/tree.rs 100% <100%> (+52.94%) ⬆️
... and 13 more

Copy link
Collaborator

@leejw51crypto leejw51crypto left a comment

Choose a reason for hiding this comment

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

lgtm

@tomtau
Copy link
Contributor Author

tomtau commented Mar 3, 2020

bors retry

bors bot added a commit that referenced this pull request Mar 3, 2020
@bors
Copy link
Contributor

bors bot commented Mar 3, 2020

try

Build failed

@tomtau
Copy link
Contributor Author

tomtau commented Mar 3, 2020

bors retry

bors bot added a commit that referenced this pull request Mar 3, 2020
@bors
Copy link
Contributor

bors bot commented Mar 3, 2020

try

Build failed

@tomtau
Copy link
Contributor Author

tomtau commented Mar 3, 2020

bors retry

bors bot added a commit that referenced this pull request Mar 3, 2020
…ixes crypto-com#1143)

Solution:
- cherry-picked changes from the planned upstream changes to the *temporary* custom secp256k1 fork
- adapted the *temporary* Rust library fork to it
- changed chain-core witness structure to use the BIP-340 encoding of public keys
- adapted client and tests to it

NOTE:
the schnorrsig proposal was stabilized as https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
the code wasn't yet merged to upstream, so it's possible that underlying cryptographic API
may change (and the corresponding Rust), but the transaction (binary/serialized) payloads
will be the same.
@bors
Copy link
Contributor

bors bot commented Mar 3, 2020

try

Build succeeded

@tomtau
Copy link
Contributor Author

tomtau commented Mar 3, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 3, 2020

@bors bors bot merged commit ad2d5cd into crypto-com:master Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants