Multisignature transaction support in the GUI

4 messages, 6 to 7 Feb 2012. Started by Gavin Andresen.

Wladimir J. van der Laan (2), Gavin Andresen, Aidan Thornton

Gavin Andresen 6 Feb 2012, 16:07 UTC permalink original

I've been testing how the Bitcoin-Qt GUI deals with multisignature transactions.

The good news is it seems to display them just fine; even my insanely-messy test wallets look reasonable.

It does not support sending multisig/BIP16 transactions, which is definitely a feature for the main network (we don't want users sending them until they will get relayed, get mined, AND will be fully verified by a large super-majority of miners).

But... to encourage more testing it might make sense to enable sending multisig transactions in the GUI if (fTestNet).

So I see two possible paths:

1) Leave the GUI as-is; require multisig testing to use the RPC interface. Note: the RPC call that make multisig sends possible (addmultisigaddress) is disabled for the main network for the 0.6 release. Don't start rolling out GUI support until the next (0.7?) release cycle.

2) Start implementing multisig support in the GUI during the 0.6 release process, enabled only for test network. This could be as simple as allowing the 35-character BIP16 multisig addresses in the 'send' dialog, to as complicated as adding/extending dialogs that let you create multisig addresses to add to your address book.

Advantage of (1) is it should mean 0.6 gets to final release faster.

Advantage of (2) is it should mean more testing of multisig, and fewer bug reports of "I added a multisig address via RPC but I can't send to it using the GUI"

My opinion: I think it is worth allowing send-to-multisig-address via the GUI (should be a very simple change to the address validation logic). But creating multisig addresses via the GUI should wait until the next release.

Wladimir J. van der Laan 6 Feb 2012, 17:27 UTC in reply to Gavin Andresen permalink original

On Mon, Feb 6, 2012 at 5:07 PM, Gavin Andresen <gavinandresen@gmail•com>wrote:

Quoted text (10 lines)

>
> Advantage of (2) is it should mean more testing of multisig, and fewer
> bug reports of "I added a multisig address via RPC but I can't send to
> it using the GUI"
>
> My opinion: I think it is worth allowing send-to-multisig-address via
> the GUI (should be a very simple change to the address validation
> logic).  But creating multisig addresses via the GUI should wait until
> the next release.
>

I think we should go with (2), changing the maximum address length and validation is very easy. We'd need to

   - Change BitcoinAddressValidator::MaxAddressLength to 35
   - The addresses are validated with walletmodel->validateAddress which in
   turn calls CBitcoinAddress addressParsed(addr) and then isValid(). Does
   this work for the new addresses?

The set of allowed characters is still the same, so BitcoinAddressValidator doesn't have to be changed.

Advanced dialogs for constructing the addresses / adding them to the address book could wait for 0.7.0.

Wladimir

Wladimir J. van der Laan 6 Feb 2012, 18:03 UTC in reply to Wladimir J. van der Laan permalink original

On Mon, Feb 6, 2012 at 6:27 PM, Wladimir <laanwj@gmail•com> wrote:

Quoted text (16 lines)

> On Mon, Feb 6, 2012 at 5:07 PM, Gavin Andresen <gavinandresen@gmail•com>wrote:
>
>>
>> Advantage of (2) is it should mean more testing of multisig, and fewer
>> bug reports of "I added a multisig address via RPC but I can't send to
>> it using the GUI"
>>
>> My opinion: I think it is worth allowing send-to-multisig-address via
>> the GUI (should be a very simple change to the address validation
>> logic).  But creating multisig addresses via the GUI should wait until
>> the next release.
>>
>
> I think we should go with (2), changing the maximum address length and
> validation is very easy. We'd need to
>

And maybe WalletModel::sendCoins has to be changed. Does CScript.SetBitcoinAddress work for the new address type?

Wladimir

Aidan Thornton 7 Feb 2012, 11:52 UTC in reply to Wladimir J. van der Laan permalink original

On Mon, Feb 6, 2012 at 5:27 PM, Wladimir <laanwj@gmail•com> wrote:

Quoted text (4 lines)

> Change BitcoinAddressValidator::MaxAddressLength to 35
> The addresses are validated with walletmodel->validateAddress which in turn
> calls CBitcoinAddress addressParsed(addr) and then isValid(). Does this work
> for the new addresses?

Should do (unless BIP 16 broke something). In fact both the GUI and RPC commands use CBitcoinAddress and IsValid to validate addresses in that way.

Quoted text (2 lines)

> And maybe WalletModel::sendCoins has to be changed.
> Does CScript.SetBitcoinAddress work for the new address type?

Again, should do, and it's used by the RPC commands for sending Bitcoins as well. Really there's not a lot that needs to be changed. I'm pretty sure even the change in MaxAddressLength is only necessary to send coins to multisig addresses on testnet, sending them on mainnet should just work.