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

Potential Issues and Proposed Solution for Implementing KIP 15: Ensuring Synchronization Between sigs and signers in Pact Commands #38

Open
Luzzotica opened this issue Mar 10, 2023 · 0 comments

Comments

@Luzzotica
Copy link
Contributor

Luzzotica commented Mar 10, 2023

As I've been implementing KIP 15 I have noticed the following potential issues.

  1. It is 100% possible that the sigs become "detached" or not match the actual signers list in the Pact Command.
  2. If a wallet holds multiple keys that a CommandSigData specifies in its sigs list, it should sign multiple times, but there isn't an QuicksignOutcome specified for this, nor it there an QuicksignOutcome if one signature succeeds but another fails.

For problem number 2, it should be easy enough to specify how the wallet should respond if multiple signatures are attached to the same CommandSigData.

I am not sure how it should respond if one sig fails and another succeeds however.
Changing it to have a list of QuicksignOutcomes associated with the CommandSigData would resolve this issue entirely.

And this issue might be something we don't care about: What's the likelihood of a wallet having two private keys required by Quicksign? Likely pretty low... But I would personally take it into account nonetheless.

For problem number 2, each library has to implement a sanity check to make sure the signers list of the Pact Command Payload actually matches the sigs list of the CommandSigData.

Floppie brought this up to me, and I agree:
It would be simpler, in the long term, to just send Pact Commands and use those to determine if more signatures are required by comparing the signers in the Pact Command Payload with the sigs in the Pact Command.

An example of a Pact Command:

{
  "hash": "",
  "sigs": [],
  "cmd": "{\"networkId\":\"mainnet01\",\"payload\":{\"exec\":{\"data\":{\"user-ks\":{\"pred\":\"keys-all\",\"keys\":[\"cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"]},\"account\":\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"},\"code\":\"(free.wiz-arena.make-offer \\\"80\\\" \\\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\\\" 1 2.0)\"}},\"signers\":[{\"clist\":[{\"name\":\"free.wiz-arena.ACCOUNT_GUARD\",\"args\":[\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"]},{\"name\":\"coin.GAS\",\"args\":[]}],\"pubKey\":\"cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"}],\"meta\":{\"creationTime\":1678470958,\"ttl\":600,\"gasLimit\":4000,\"chainId\":\"1\",\"gasPrice\":1e-8,\"sender\":\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"},\"nonce\":\"2023-03-10T17:55:57.659484Z\"}"
}

All I have to do is send this to a wallet, they can deserialize the json, and check the signers list to see if they have a matching public key.
If they do, they sign, and add their sig to the sigs list.
Then return the Pact Command with the hash and new sigs attached:

{
  "hash": "hashabc",
  "sigs": [{
    "sig": "sigxyz"
  }],
  "cmd": "{\"networkId\":\"mainnet01\",\"payload\":{\"exec\":{\"data\":{\"user-ks\":{\"pred\":\"keys-all\",\"keys\":[\"cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"]},\"account\":\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"},\"code\":\"(free.wiz-arena.make-offer \\\"80\\\" \\\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\\\" 1 2.0)\"}},\"signers\":[{\"clist\":[{\"name\":\"free.wiz-arena.ACCOUNT_GUARD\",\"args\":[\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"]},{\"name\":\"coin.GAS\",\"args\":[]}],\"pubKey\":\"cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"}],\"meta\":{\"creationTime\":1678470958,\"ttl\":600,\"gasLimit\":4000,\"chainId\":\"1\",\"gasPrice\":1e-8,\"sender\":\"k:cf415c73edb4666a967933bddc2e6c4a6e13b8ec0566e612b9f3cbe4a4d8506e\"},\"nonce\":\"2023-03-10T17:55:57.659484Z\"}"
}

Granted, this doesn't allow for what QuicksignOutcome allows for, but I feel like you can likely create some kind of structure that works in exactly the same way to give errors/feedback to the user.
Building things in this manner would:

  1. Remove the potential of the sigs to get desynced from the signers
  2. Make it possible for dApps to immediately use the result with the Pact API.

Both of the above are current hurdles with Quicksign as it stands.

Perhaps I should make a new KIP for the above...

@alber70g alber70g changed the title KIP15 Problems Potential Issues and Proposed Solution for Implementing KIP 15: Ensuring Synchronization Between sigs and signers in Pact Commands Jun 8, 2023
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

No branches or pull requests

1 participant