wallet: derive all keys in generateNonce for multisig

[rithvikvibhu]

Multisig accounts will generate different nonce given the same nameHash, p2sh address and value as only "own" public key is used. This PR derives public keys from all keys and picks the lexicographically smallest one so all parties generate the same nonce. Implements @pinheadmz's suggestion: https://t.me/hns_tech/70247

Tests

2 tests are added, one for regular pubkeyhash accounts and another for 2-of-2 multisig. Assertions compare to specific hardcoded values to ensure that this PR is not a breaking change (at least for regular wallets).

Running the test on master passes the first (pubkeyhash account), but fails on the second (multisig) because alice and bob generate different nonces.

[coveralls]

Coverage increased (+0.2%) to 68.199% when pulling 5f485566e73884f764e845d468507d98a0069f9e on rithvikvibhu:multisig-generatenonce into 234a5974dc4df3b83f5945c101d6fca709f36810 on handshake-org:master.

[rithvikvibhu]

I wonder if this should be published as (maybe very short) HIP since it's an application-level standard we'll need for interoperability with new HNS wallets.

:+1: Will have a draft ready along with Bob's multisig PR

[pinheadmz]

It just occurred to me that this is also a potentially breaking change. A multisig party may have already placed a bid with their own key's nonce and instead of the group key like you've implemented. Attempting to recover that wallet using importnonce after this PR is merged would not work.

I'm not sure what the prettiest code would be but to be totally covered I think the logic for importnonce may need to derive both old and new nonces if the wallet is multisig. As long as both nonces are stored in the db, the reveal action will find the correct one and finish the TX.

[rithvikvibhu]

I looked for all places generateNonce (and by extension generateBlind) are used:

• Get Nonce for Bid HTTP API (docs)
• importnonce RPC method
• makeBid in wallet

So yes, it's a breaking change when:

• it is a multisig account, and
• has placed bids that: (a) aren't revealed yet, and (b) can still be revealed, and
• the wallet is recovered from seed / ledger after the bid was placed, and
• own key is not the first key lexically (but unlikely, so we can ignore this point)

Storing both nonces in db doubles what's stored on disk for all current multisig wallets as well as those created in the future. Instead, how about one of these (in the order I prefer them, first being the best of them):

Option 1:

• new param to importnonce like multisig-legacy: bool that uses the old generateNonce (defaults to false)
• same change to the HTTP api
• Maybe: when this is true, we generate both old and new. If false, then only new.

Option 2:

• a script/api-method/function/whatever that reads all existing BidBlinds and generates old nonces for them
• useful if one wants to importnonce (many bids) without change and in the end call this to generate and store old nonces

Option 3:

• no invisible upgrade path, warn users not to have active and unrevealed bids (in multisig) before update
• I'm considering this as an option (even if it's last) because multisig isn't widely used yet and regular wallets aren't affected
• in case a wallet really needs to be recovered, a script that does option 2 can be run separately

I like the first one. Was thinking of a wallet migration or similar, but it wouldn't work here because it affects new wallets, whereas migrations are for existing ones.

[pinheadmz]

I like the direction of option 1, just adding a --legacy flag or something. I think what that could do is generate and store nonces using every public key in the multisig set. That would ensure that ANY member of the party would be able to repair these bids, assuming they know the secret bid value.

We may even just want to do this without a flag (by default) since it is a bit of an edge case anyway. That would just be for rpc/httpimportnonce and not all generateNonce's.

Storing both nonces in db doubles what's stored on disk for all current multisig wallets as well as those created in the future.

I'm not too worried about storing a few extra hashes in the DB for now. Truth is, the wallet DB stores a ridiculous amount of extra data (e.g. everyone else's bids on auctions you participated in, future bids on re-opened names, etc) and pruning this garbage will be another big task that will increase in urgency as time goes on...

[rithvikvibhu]

Okay so now the way it works is:

Note: Doesn't affect pkh accounts, only multisig.

Normal behavior (makeBid, etc.):

• Smallest public key is used to generate nonce
• Only 1 blind is stored based on that nonce

Importnonce RPC/HTTP behavior (forAllKeys is true):

• generateNonce returns N nonces (one for each key)
• generateBlind stores N blinds, but returns the own-pub-key blind only
• This way, the importnonce response doesn't change and is backward compatible

[pinheadmz]

Right now flags have 7 bits unused, we can take 2nd bit for this. We can introduce new flag in account - e.g. this.multisigBlinds = true

@nodech as discussed, this doesn't solve anything because after this is merged, we can't guarantee everyone in every multisig party is running updated software. So in a recovery scenario (with importnonce) we still need to try all nonces.

[rithvikvibhu]

Addressed all comments and cleaned up commits.