-
Notifications
You must be signed in to change notification settings - Fork 375
CIP-0112 | Changes to signature checking #1105
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
Conversation
Currently, the `txid`, i.e. the hash of the transaction body, is signed. We propose this is changed to all keys instead signing the hash of the concatenation `txid_and_obs = hash (txid ++ required_observers_hash)`. This would also require that the `TxBody` type contain the hash of the `required_observers` field *in addition* to the deserialized contents of the field, e.g. as a separate field, or as a pair `(required_observers_hash , required_observers)`. Both `required_observers_hash` and `txid_and_obs` should be computed during transaction deserialization and included in the deserialized `TxBody`.
|
thanks @polinavino ... To help editors assess how to confirm this at tomorrow's CIP meeting |
|
@rphair Thanks for mentioning me. I have not encountered this PR before, but I will review it and bring my thoughts to tomorrow's CIP editors meeting. At first glance, it looks pretty cool! |
|
thanks @GeorgeFlerovsky & I meant that your review was helpful on CIP-0112 itself 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll keep an eye out for further confirmation... but in the meantime it seems this is a prerequisite to finalise the candidate CIP-0118 and therefore I would see this update as an essential dependency unless there's an objection posted:
|
I've already suggested this to @polinavino during the meeting, but here is my opinion on the record. I think this change deserves its own CIP, since this change is not necessary for the goal of this CIP and is not clear whether the benefit it provides is worth the complexity, which needs to be discussed properly. In particular has it been decided that we need to support this use case:
I understand that this is what @polinavino is working on at the stage of research, but it has not been decided whether this is really necessary (eg. there is no CIP about it) Also this change needs to be described in more detail, in particular what changes to CDDL is necessary and what checks need to be implemented at the ledger level. I can sort of see what is needed, but for the benefit of others I believe it would be worthwhile to bring more clarity to why and how. |
|
@polinavino it seems to me from @lehins's #1105 (comment) that our best course of action for the CIP meeting today would be to triage this |
|
I agree with the sentiment that this change is better suited for a separate CIP, especially if the primary use case is the light-client system mentioned in this ledger working group meeting. I'd appreciate more clarification on the target user for this functionality. I have a hard time believing that any user, even a beginner, would be comfortable spending funds without visibility into their wallet's state. After all, even grandmas stay up-to-date on their bank statements. If things have changed since that meeting, all the more reason for a separate CIP. |
|
Separate CIP makes sense! I will do that. As for justification of the change itself, here are some thoughts: (ii) anything more than the most basic payments or withdrawals from people/contracts is best done using a full or light node node (bootstrapped either from genesis or a more recent state such as a Mythril one) (iii) if a device has access to the correct wallet state of some user, that is either because this device (1) is a node (no LC design needed), OR (2) saved the state from an earlier time, and it happens to not have changed, OR (3) has queried a full node for their state. So, (2) requires that there is no shared wallet state across multiple machines, which seems too restrictive. Then, (3) - based on (ii), we should assume the goal of an LC is mostly to construct a transaction. The LC design we propose, where a node constructs a transaction for you, with the signature change presented here, sidesteps any need for LC querying anything, or checking proofs that the query results are correct |
|
@polinavino we're talking about this at the CIP meeting now and will mark this It sounds like you already have the motivation sketched out directly above & others at the meeting would also like to see what you can write about how those motivations are achieved. Also we would hope @colll78 will look at the new submission from a security / audit point of view & we'll be sure to tag him when the new CIP PR comes out. |
|
Made a CIP instead #1110 |
The purpose of this PR is to allow observer scripts to be treated as intents, which are, in some sense, separate from the transaction which has been constructed to satisfy them. Making the proposed change will allow for super-lightweight proof of intent fulfilment by a transaction, without ever having to inspect the contents of its fields except the
required_observersfield.The change:
Currently, the
txid, i.e. the hash of the transaction body, is signed. We propose this is changed to all keys instead signing the hash of the concatenationtxid_and_obs = hash (txid ++ required_observers_hash). This would also require that theTxBodytype contain the hash of therequired_observersfield in addition to the deserialized contents of the field, e.g. as a separate field, or as a pair(required_observers_hash , required_observers). Bothrequired_observers_hashandtxid_and_obsshould be computed during transaction deserialization and included in the deserializedTxBody.(link to added section)