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

Kycfile contract #204

Merged
merged 17 commits into from
Sep 3, 2019
Merged

Kycfile contract #204

merged 17 commits into from
Sep 3, 2019

Conversation

lawlawlaw
Copy link

  • New RPCs:
    -- dumpkycpubkeys
    -- removekycpubkey

  • Fixes to onboarding with contractintx.

qa/rpc-tests/test_framework/util.py Outdated Show resolved Hide resolved

void CWhiteList::sync_whitelist_wallet(){
boost::recursive_mutex::scoped_lock scoped_lock(_mtx);
std::vector<CPubKey> keysNotFound;

Choose a reason for hiding this comment

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

This vector is only used inside sync_whitelist_wallet. why do we need two methods for this?

Choose a reason for hiding this comment

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

I don't see where keysNotFound is used.

Copy link
Author

Choose a reason for hiding this comment

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

I may add an RPC for this if needed - keysNotFound will be used for that.

src/wallet/wallet.cpp Outdated Show resolved Hide resolved
CPubKey pubKey;
if(Params().ContractInTx()){
pubKey=key.GetPubKey();
} else {

Choose a reason for hiding this comment

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

This should be if (Params().EmbedContract() &! bEncryption) no? That is condition for applying tweaking in GenerateNewKey() in wallet.cpp. I am not sure why bEncryption was added.

Copy link
Author

@lawlawlaw lawlawlaw Sep 3, 2019

Choose a reason for hiding this comment

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

Actually GenerateNewKey() tweaks the key under the following condition:
Params().EmbedContract() && Params().ContractInTx() &! contract.IsNull()

I have updated accordingly.

"bEncryption" is key dependent and is "true" for generating keys in the "encryption" HD path as these keys do not need to be tweaked.

Choose a reason for hiding this comment

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

Sorry was checking old code. It tweaks when !Params().ContractInTx() but your statement now will tweak even when embed contract is disabled.

Copy link
Author

Choose a reason for hiding this comment

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

It doesn't make any difference because without tweaking pubKey and derivedPubKey are the same, so I've removed the check and just used derivedPubKey.

nkostoulas
nkostoulas previously approved these changes Sep 3, 2019
@lawlawlaw lawlawlaw merged commit 33b9140 into master Sep 3, 2019
@nkostoulas nkostoulas deleted the kycfileContract branch September 11, 2019 10:29
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.

2 participants