-
Notifications
You must be signed in to change notification settings - Fork 64
Second signature registration transaction should update secondSignature flag for account - Closes #1147 #1151
Conversation
@@ -209,6 +210,8 @@ export class SecondSignatureTransaction extends BaseTransaction { | |||
protected undoAsset(store: StateStore): ReadonlyArray<TransactionError> { | |||
const sender = store.account.get(this.senderId); | |||
const { secondPublicKey, ...strippedSender } = sender; |
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.
const { secondPublicKey, ...strippedSender } = sender; | |
const { secondPublicKey, secondSignature, ...strippedSender } = sender; |
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.
@shuse2 I think we need the properties to be present for stateStore to handle them correctly so instead changed to de-structuring as we discussed.
@@ -200,6 +200,7 @@ export class SecondSignatureTransaction extends BaseTransaction { | |||
const updatedSender = { | |||
...sender, | |||
secondPublicKey: this.asset.signature.publicKey, | |||
secondSignature: true, |
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.
It should be a number, with values either 0 or 1.
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.
Done!
const { secondPublicKey, ...strippedSender } = sender; | ||
const strippedSender = { | ||
...sender, | ||
secondPublicKey: undefined, |
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.
In Lisk-core, on undo action, the secondPublicKey property is assigned with the value null
. Therefore, I think we should keep it consistent.
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.
@SargeKhan We have a TS lint rule that forbids null and favours undefined should I skip it in this case?
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.
Yeah, I guess we have to do it in this particular case.
…transaction-should-update-secondsignature-flag-for-account
What was the problem?
How did I fix it?
How to test it?
Build should be green
Review checklist