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

Sign single account #4957

Merged
merged 4 commits into from Dec 16, 2020
Merged

Sign single account #4957

merged 4 commits into from Dec 16, 2020

Conversation

sqrrm
Copy link
Member

@sqrrm sqrrm commented Dec 15, 2020

This a proposed solution to bisq-network/growth#212

The user to get signed copies the account data and sends it over whichever channel, keybase for example. With this change the arbitrator can sign this account.

Step 1, user copies account data (ctrl+c to copy AccountAgeWitness and PubKey to clipboard)
image

Step 2, arbitrator imports data (under accounts view, open signing tab with ctrl+i, in signing tab, open specific account signing window with ctrl+p)
image

Step 3, arbitrator signs with their private key (this could be helpful for devs that want to test signed accounts as well)
image

Step 4, done
image

I know this is very pretty, but please don't ask me to implement all the UI stuff due to this flash feature.

Use ctrl+c to copy accountagewitness and pubkey to clipboard in a format to
be used by arbitrator to sign the account
Change the specific sign window to sign an imported account in the form
of accountAgeWitnessString,pubKeyString. The SignSpecificWitness never
worked due to missing pubKey data and this is a way to import the missing data.
chimp1984
chimp1984 previously approved these changes Dec 15, 2020
Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

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

utACK, some nits and suggestions but looks ok... not reviewed UI part much

@@ -62,6 +62,8 @@ public void initialize() {
accountAgeWitnessService.getAccountAgeWitnessUtils().logSigners();
} else if (Utilities.isCtrlShiftPressed(KeyCode.U, event)) {
accountAgeWitnessService.getAccountAgeWitnessUtils().logUnsignedSignerPubKeys();
} else if (Utilities.isAltOrCtrlPressed(KeyCode.C, event)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Cmd+c is a very common key command, not sure if its better to use a more exotic one so that user do not trigger that accidentally. Prob. no harm by doing that, but confusing...

Copy link
Contributor

Choose a reason for hiding this comment

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

As its only used for fiat, better move it to fiat class instead of base class

Copy link
Member Author

Choose a reason for hiding this comment

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

I like adding it here, and I like ctrl+c since it's the common copy key and that's what we're doing here, copying the account info to clipboard.

Copy link
Contributor

Choose a reason for hiding this comment

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

it opens a popup window right?

Copy link
Contributor

Choose a reason for hiding this comment

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

or just copy to clipboard?

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry have not looked too close to the code base....

Copy link
Member Author

Choose a reason for hiding this comment

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

Just copy to clipboard, no need for popup window.

Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

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

utACK

Will test it in detail during release testing.

@ripcurlx ripcurlx added this to the v1.5.2 milestone Dec 16, 2020
@ripcurlx ripcurlx merged commit 0789180 into bisq-network:master Dec 16, 2020
@sqrrm sqrrm deleted the sign-single-account branch May 3, 2021 12:41
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.

None yet

3 participants