Duplicated signatures in transactions - Closes #2254 #2275
Conversation
|
@nazarhussain, @SargeKhan, @ManuGowda Please review. I'll write unit tests for new functions after resolving all feedback from the review. |
modules/multisignatures.js
Outdated
| /** | ||
| * Gets transaction from transaction id and add it to sequence and bus. | ||
| * Check is provided signature is valid |
There was a problem hiding this comment.
check if the provided signature is valid
modules/multisignatures.js
Outdated
| // If publicKey is provided we can perform direct verify | ||
| if (signature.publicKey) { | ||
| // Check if publicKey is present as member of multisignature account in transaction | ||
| if (members.indexOf(signature.publicKey) === -1) { |
There was a problem hiding this comment.
maybe? members.includes(signature.publicKey)
modules/multisignatures.js
Outdated
| return setImmediate(cb, 'Transaction not found'); | ||
| __private.isValidSignature = (signature, members, transaction) => { | ||
| let isValid = false; | ||
|
|
There was a problem hiding this comment.
const { signature, publicKey } = signature;
and reuse the variable
There was a problem hiding this comment.
I cannot do that because I log the entire signature object later.
modules/multisignatures.js
Outdated
| * @param {function} cb - Callback function | ||
| * @returns {setImmediateCallback} cb, err | ||
| */ | ||
| __private.processSignatureForMultisignatureCreation = ( |
There was a problem hiding this comment.
rename function to processSignatureForMultisignatureAccountCreation
modules/multisignatures.js
Outdated
| return setImmediate(cb, 'Failed to verify signature'); | ||
| } | ||
| // Check if received signature already exists in transaction | ||
| if (transaction.signatures.indexOf(signature.signature) !== -1) { |
There was a problem hiding this comment.
use of includes here as well
| minimum: 2, | ||
| }); | ||
|
|
||
| // Create signatures (strings) |
…o error but sender = undefined
…tion returns no transaction
|
Unit tests completed, please review @nazarhussain, @SargeKhan, @ManuGowda. |
Duplicated signatures in transactions - Closes #2254
Duplicated signatures in transactions - Closes #2254
What was the problem?
Duplicated signatures in transactions (send from multisignature, multisignature registration). More details in #2254 (comment)
How did I fix it?
Refactored
processSignaturefunction inmultisignaturesmodule, execute all the logic throughbalanceSequenceHow to test it?
Run test suite.
Review checklist