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

Improve Proof building tool #229

Merged
merged 29 commits into from
Aug 24, 2022
Merged

Improve Proof building tool #229

merged 29 commits into from
Aug 24, 2022

Conversation

PiRK
Copy link
Collaborator

@PiRK PiRK commented Aug 19, 2022

This PR refactors the proof building tool to allow creating proof templates (with no stakes), viewing and editing existing proofs, adding stakes to an existing proof.

Until now, we assumed that the wallet signing the proof and the wallet signing the stakes was the same. From this PR, the assumption will be that they are normally separate wallets. What seems to make most sense is to create a dummy wallet to build the proof, separate from the wallet containing the stake. This removes the need of having to export any private key from the staking wallet (which could be an offline wallet restored from the seed from a hardware wallet).

This work also prepares the way to collaboratively build proofs, with inputs from multiple coinholders.

It introduces a file format for exchanging proofs, a .proof file, which contains a hexadecimal proof. This proof may have an invalid master signature during some stages of the building process, namely after the staking wallets adds signed stakes to the proof and before the proof signing wallet can update the master signature to include the new stakes.

@PiRK PiRK force-pushed the proof_template branch 4 times, most recently from b5fe732 to c184354 Compare August 23, 2022 09:04
@PiRK PiRK changed the title [WIP] Refactor Proof building tool Improve Proof building tool Aug 23, 2022
@PiRK
Copy link
Collaborator Author

PiRK commented Aug 23, 2022

Ideas for future improvements:

  • use window menus instead of buttons, to make the window less complex
  • make it impossible to mess up the stake gathering process, to merge unrelated proof (maybe load all files at once and fail if the pubkey and expiration time are not matching)

@PiRK PiRK force-pushed the proof_template branch 2 times, most recently from b8d7227 to 03aaea6 Compare August 24, 2022 08:18
PiRK added 22 commits August 24, 2022 10:20
In the previous Proof format, the stake key used to sign the proofid, which depended on the whole list of stakes. So we needed to keep a list of stake signers, and sign the stake only when they were all added.

The new proof format is now designed to allow adding new stakes to an existing proof without requiring all the stake signatures to be updated. The stake key no longer signs all the other stakes. We can now sign the stakes as soon as they are added.
There is a Stake and a SignedStake object. This makes it clear that the list contains the latter.
This allows to create a proofbuilder with exact same parameters and same existing stakes, and than add more stakes.

The master private key must be provided again, as only the corresponding public key is available in the proof.

Add a test (test result generated using the node buildavalancheproof RPC).
Add also a stake_commitment attribute to the `Proof` class, so it is easy to verify the stakes in that proof.
This adds sanity checks to ensure the current code and tests are good, before changing the ProofBuilder to take an address rather than a script in the next commit.
This is in line with the buildavalancheproof RPC. If we ever need to support arbitrary scripts or pubkeys in the future, it is already possible using a ScriptOutput or a PublicKey object, even though the "payout_address" name does not really reflect this (fixme).
Instead of passing utxos to the widget's __init__, add utxos using a dedicated method.
This will enable adding additional coins once the widget is already built.
This allows to remove the weird overloading of exec_ and the special do_execute flag, as we no longer need to test the utxos in __init__.
Also rename AvalancheProofWidget to AvalancheProofEditor
Disabling the expiration time will cause it to be 0 in the proof. The calendar and timestamp combobox will be disabled.
This loads coins from a json file.
… available

This is useful for adding stakes extracted from an existing proof, without need to find the private key again and redo the signature. This will be especially useful when building proofs collaboratively, with different wallets adding their stakes to a proof templates, and later merging the proof in a master wallet that does not have the utxo keys.
This implements loading an existing proof.
It makes more sense than the add_utxo method which has basically weak typing for parameters.
We want to be able to build  proofs with no stake to be used as proof templates for other wallets to load and stakes.
Previously we loaded them to show them in red in the utxo table, and later ignored them when building the proof.
Excluding them early is easier, and will allow storing Stake objects directly instead of vaguely defined dictionaries.
This removes boilerplate code attempting to analyze the dict objects used previously. Now, the dict object will only be used initially when loading coins from a file or building from coins in this wallet, and it will be converted internally into a Stake + Key (we need to wait for the rest of the proof data to be final before signing the stake).

Remove also the coin freezing feature on proof building. The coins will likely not belong to the wallet building the proof.
…anche proof editor"

The new workflow is not to open the proof editor, then click the "load coins" button.
This saves a proof to file, in hexadecimal text format.
Also change the buttons layout to be horizontal below the proof display.
Also use a regular button for the "generate delegation" button. The link looks weird.
…h invalid sig)

This changes the proof builder to make it accept a master pub instead of a master private key.
The resulting signature will be invalid.

This is to allow people to add signed stakes to a proof without knowledge of the master key. Such an invalid proof can be used to gather stakes from multiple users, and merge all the proofs in the master wallet (who can then produce a correct signature).
In such a case, if the public key is available, a proof can be produced with an invalid signature. This is useful for gathering signed stakes in invalid proofs (from other wallets) and later merging the resulting proofs  in the master wallet.
This should enable merging proofs with identical stake commitment data (master key and expiration time) but different stakes.
The use case is:
 - a master wallet creates a proof template with no stakes (but all other data otherwise correct) and save it to a .proof file
 - multiple other wallets load this proof and add signed stakes to it from coin files, and save the result to a .proof file.
 - the master wallet loads one of the proof files (possibly the initial template), and then merge the stakes from all other files.
…utton

There is no need for a "reject" status for these dialogs for now, they are pretty independant from the main window.
…vate master key

Creating an unsigned proof is a standard procedure when signing the stakes with a different wallet, for the proof to be signed again later in the master wallet.
So make the warnings less alarming, mention that it is OK to just sign stakes.
Print good signatures in green, bad signatures in red. Add labels indicating the status of signatures below the proof display.
In the proof editor, the "load proof" button now load a dialog accepting a copy-pasted string or loading from a .proof file.

In the delegation tool, there is now a button "Load from file" to open a .proof file.
In the serialization, I forgot to encode the number of stakes and the pubkey length. Fix this and add an assertion to avoid regressions.
…licable

When signing stakes in a different wallet than the wallet controlling the master key, the proof signature will be all 0s. Reflect this in the file name when saving the proof, to help with managing the files during the multi-step signing procedure.
@PiRK PiRK merged commit 2bb17b5 into Bitcoin-ABC:master Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant