stellar / stellar-protocol

Developer discussion about possible changes to the protocol.
523 stars 304 forks source link

Mutlisignature protocol amelioration #120

Open MisterTicot opened 6 years ago

MisterTicot commented 6 years ago

Current behavior

When a transaction is signed by more legit signers than required, it will be rejected with 'tx_bad_auth_extra' error.

Proposed behavior

The transaction should be validated and each legit signer should be recorded into the blockchain; Or at least the transaction should get validaded.

Rationale

There's two distinct propositions here:

  1. A transaction with enough valid signatures should always be validated:

    • First of all rejecting valid transactions is a non-sense
    • Rejecting transaction with more-than-needed signers force wallets willing to implement multisignature to add routines to check that there's not "too much" signers and to strip them out. This routine could be implemented at stellar-core level, saving wallets implementors from this redundant task and making them more likely to implement it right in the first place.
  2. Additional signers should be recorder in the block chain:

    • There's use cases where it does matters who signed, beyond the fact that the threshold have been met. For instance: voting system, legal contracts, proof of presence. Those are commons use of blockchain systems that would easily and cheaply get supported by Stellar if this proposal get accepted. Supporting those use cases would be beneficial for Stellar and its ecosystem.
cesarmak commented 6 years ago

This is particular true when a transaction has multiple operations (and/or using channels).

Example

P has 3 signers: P, Q, R (each with weight=1) For P, all thresholds = 2 (i.e., 2 signers needed)

  1. R conducts the tx (R as a channel)
  2. add Operation: P pays Z, 1XLM
  3. add Operation: Q pays Z, 1XLM
  4. sign with P, Q, R
  5. submit tx -->tx_bad_auth_extra

I can't think of actual use case of the above scenario yet, but it seems the system deemed the 1st operation invalid.

MonsieurNicolas commented 6 years ago

Reason we did this was because of the quadratic nature of signature verification (which is the most CPU intensive thing we deal with before accepting transactions in consensus - the hints help but they don't help making the worst case). I agree this may be more burden than what it's worth. It may make sense to reconsider this the next time we look into changing the fee structure: I think that there should still be an incentive to submit minimal transactions to the network.

vogel commented 6 years ago

@cesarmak your example should get executed if only P and Q signed this transaction.

cesarmak commented 6 years ago

@vogel yes, I've tested a bit on the marginal case; just wanted to point out that some possible scenarios could be rejected.

So, is it the proper way to query Horizon on the Account Endpoint, and do a weight checking with the "threshold" & "signers", before trying to submit the transaction?

vogel commented 6 years ago

Yes, that is the proper way.

theaeolianmachine commented 5 years ago

@MonsieurNicolas is it worth reviving this discussion? I agree it's a weird behavior that affects usability, but it feels mildly painful as opposed to detrimental (especially if changing the behavior would hurt performance a good bit). @graydon do you have any thoughts?

If we'd be open to discussing this further, I'd like to push this to be an official draft and put it on the docket.

MonsieurNicolas commented 5 years ago

I have nothing else to add to this thread at this point

theaeolianmachine commented 5 years ago

@MonsieurNicolas do you mean "yes, I think this should be revisited but I don't have the bandwidth to look into it/write a CAP" or "no, I don't think the cost of implementation is worth the benefits, and we should close this out and do a better job documenting this."