stellar / stellar-protocol

Developer discussion about possible changes to the protocol.
522 stars 303 forks source link

Proposal: Extra signer keys field in a transaction #89

Open robdenison opened 6 years ago

robdenison commented 6 years ago

This proposal would add a new extraSignerKeys field to the transaction header that would allow a transaction to specify extra conditions that must be satisfied for the transaction to be valid.

The field would be an array of SignerKeys. (SignerKey is the type of the key property used in the Signer object (which in turn is used in the SetOptions operation). These signer keys could be public keys, sha256 hashes, or preauthorized transaction hashes (although the latter would be impossible to satisfy, since this field is itself incorporated into the transaction hash).

The transaction envelope would have to include signatures or preimages satisfying each of those extra signers, in addition to those satisfying the signers on the source account.

The total number of signers (between both signers on the account and extra signers on the transaction) cannot be higher than 20.

If one of the signers is not satisfied, the transaction would fail with code TX_BAD_AUTH.

Rationale

This allows preauthorized or presigned transactions to specify additional conditions that must be satisfied at the time that they are spent.

Something like this is necessary to implement the hashed timelock contracts (HTLCs) used in multi-hop payment channel networks like Lightning. These require presigned or preauthorized transactions that can only be spent if a certain hash preimage is provided. Currently, you can set a hash preimage as a signer on an account, which is enough to implement a single HTLC in a payment channel. However, it appears to currently be impossible to extend this solution to multiple HTLCs in a single channel, because each hash preimage should only be allowed to authorize one specific transaction.

This could also be useful for escrow-like functionality, where a semi-trusted third party has approval power over one specific transaction. Others have noted the absence of this functionality: https://github.com/stellar/stellar-core/issues/1329.

Implementation

Transactions are already designed to be extensible:

struct Transaction
{
    ...

    // reserved for future use
    union switch (int v)
    {
    case 0:
        void;
    case 1:
        SignerKey* extraSignerKeys<20>;
    }
    ext;
};

Signatures satisfying each of the extra signer keys must be provided in the signatures field of the transaction envelope, along with the signatures required for the source account and any operations involving other accounts.

These signatures would be checked at the same time the signatures for the source account are checked.

Alternatives

Backwards compatibility

This is a breaking consensus change (although it's probably what we would call a "soft fork" in the blockchain context).

Since this adds a field and does not change or remove any fields, it probably will not break any applications or user code, although it will require updates to the SDKs.

JeremyRubin commented 6 years ago

I think the proposed Satisfy semantics are better.

We want the conditions checked to result in an invalid transaction, not in a failed transaction so we should check them in pre-processing. No transaction should ever have a failed Assert.

Generally, I think we should implement this as an operation called Assert.

Each Assert has 1 byte indicating the assert type, and then an arbitrary byte-vector (subject to normal transaction size constraints) assertion program. Perhaps we should also include a bool for if the condition should be true or false...

For example:

AssertType KEY_SIGNED = 0; AssertType MIN_BALANCE = 1; AssertType UNKNOWN = 255;

Then, we can do something like:

Assert(KEY_SIGNED, ); Assert(MIN_BALANCE, 10 lumens);

Unknown assertion types should always return true.

This way, as long as you known that a v blocking set is guaranteeing the new op, it will be safe to include. (They can even do a 'bounty' on this, by releasing a transaction to you with Assert(KEY_SIGNED, random()); which could only end up in a ledger if they don't check the condition). Of course, a full version update will also work.

This will enable more rapid development of new assertions.

Different types of assertion:

Balance Checks Signatures Sequence Number of Arbitrary Accounts Presence of trustlines on another account Existence of another account Self-Referential Assertions (All accounts I'm sending to exist)

etc...

robdenison commented 6 years ago

I agree we need the conditions to result in an invalid transaction, not a failed one.

Doesn't particularly matter to me whether this goes in an operation or the header. Executing some operations as part of the preprocessing phase seems a little unusual (as far as I know that's not currently done?), but not fatally so (as long as having to iterate over 100 operations at that step isn't a DoS vector, which it probably isn't—you have to parse the whole transaction anyway).

Allowing asserts to check arbitrary bits of state (like sequence numbers on other accounts and existence of other accounts) definitely seems like a DoS vector. You could have a transaction with 99 asserts about different accounts, and only the last one fails. The validator would have to check all of those in the preprocessing phase before eventually invalidating the transaction (earning no fees).

I don't think we should develop assertions separately from authorization conditions on accounts. Any check that can be done in an assertion should also be possible to set as a "signer" on the account. I definitely do think we should make those more flexible and powerful, but I think that's out of scope of this request.

JeremyRubin commented 6 years ago

Operations actually are currently executed partially...

The transaction frames have two phases, doApply and doCheckValid. The appropriate place for this would be in doCheckValid.

I agree that there is a slight DoS vector -- but it isn't actually that bad, because you don't propagate txns that fail at doCheckValid. So it affects roughly one node, and not every node. Of course, with things like balance checks this is less true. But I think one DB access should be less expensive than a sig validation.

I slightly disagree that 'I don't think we should develop assertions separately from authorization conditions on accounts'. There are certain types of assertion that make sense at the transaction level but not at the account level, and there is a need to instantiate simultaneous transactions with differing conditional assertions. For example, if I have TX A and TX B at sequence 1110. A asserts 'Balance is below 10' and B asserts 'Balance is >= 10'. Adding that condition as a signer to an account doesn't quite make as much sense as at the transaction level because we don't want them to also permit some TX C.

It feels like a weak difference, but I think I can generalize it by stating that 'all signers should require some secret to be revealed' (if you agree with the ontology that any signature is a type of secret), whereas assertions just check some fact.

robdenison commented 6 years ago

The transaction frames have two phases, doApply and doCheckValid. The appropriate place for this would be in doCheckValid.

My mistake, OK, then an Assert operation makes a lot of sense. (Except one more question—where do you put the signature or preimage itself? It seems like it has to go in the decorated signatures on the transaction frame, but that complicates the logic that checks that all signatures in a transaction are used.)

I still am very wary of allowing these assertions to depend on ledger state. doCheckValid checks are not supposed to depend on ledger state, in part because ledger state can change as a result of the execution of previous operations. But that's separate from the question of whether this sort of check should be in the operations or in the header; I'm comfortable with the former now.

But I think one DB access should be less expensive than a sig validation.

I don't think that's true in terms of latency, and signature checks are easier to parallelize too.

It feels like a weak difference, but I think I can generalize it by stating that 'all signers should require some secret to be revealed' (if you agree with the ontology that any signature is a type of secret), whereas assertions just check some fact.

This isn't true of preauthorized transactions, which don't involve any secret.

And I could definitely imagine other cases where signers might want to check some fact. For example, you might want a given signing key to only become valid once an account balance goes below a certain level, or allow a signing key to mature after a certain time (see #90). This could be done by having multiple keys with different thresholds (or even more easily and flexibly if we get better mechanisms to combine signers monotonically).

theaeolianmachine commented 5 years ago

Hey @robdenison, would you mind throwing us a PR for your draft? Take a look at the readme in core/ for more on the process so that we can revive the conversation and see what's next.