Always use full commitment #1818
Conversation
| @@ -897,8 +901,6 @@ class AccountUpdate implements Types.AccountUpdate { | |||
| */ | |||
| requireSignature() { | |||
| let { nonce, isSameAsFeePayer } = AccountUpdate.getSigningInfo(this); | |||
| // if this account is the same as the fee payer, we use the "full commitment" for replay protection | |||
| this.body.useFullCommitment = isSameAsFeePayer; | |||
There was a problem hiding this comment.
previously there were two different ways we ensured that it's not replayed:
- if the account was the same as the fee payer, we used the full commitment
- otherwise, we incremented the nonce and set a nonce precondition
I like the simplification of just always using the full commitment (my initial idea on why not to do that was flexibility of signing with one key without yet having the fee payer sign; but now I think the simplicity is more important)
What this also means is that we don't need to do the nonce increase in this function depending on isSameAsFeePayer.
Instead, I would do the following:
- add a
booleanoptional parameter to this functionnoFullCommitment, if it's true do the nonce increment instead - remove the
isSameAsFeePayerreturn value fromgetSigningInfo
|
|
||
| // useFullCommitment = true by default, prevents replay attacks and similar issues | ||
| // the developer can still modify this to false if needed later on | ||
| body.useFullCommitment = Bool(true); |
There was a problem hiding this comment.
I'm not sure about this, how about setting this to true in requireSignature() instead of always? it's a bit weird to me that the default account update differs from the natural dummy AU with no apparent reason
There was a problem hiding this comment.
or in setLazySignature?
There was a problem hiding this comment.
well or maybe we should remove setLazySignature since it no longer holds a private key so no real purpose
Why? This prevents transactions from being replayed. Deletable accounts will also benefit from this change in the future
closes #1798