-
Notifications
You must be signed in to change notification settings - Fork 458
_verifyTerminatedStateAccountsCommon doesn't need to be inside loop #9079
_verifyTerminatedStateAccountsCommon doesn't need to be inside loop #9079
Conversation
Codecov Report
@@ Coverage Diff @@
## release/6.0.0 #9079 +/- ##
================================================
Coverage ? 83.46%
================================================
Files ? 604
Lines ? 22796
Branches ? 3365
================================================
Hits ? 19027
Misses ? 3769
Partials ? 0
|
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.
Looks good,
- I think we need to pass ownChainID i.e.,
const mainchainID = getMainchainID(ctx.chainID);
Another small change,
- https://github.com/LiskHQ/lisk-sdk/blob/c9c1ecc7f71b41edabb2919ced2c7c51f3192a04/framework/src/modules/interoperability/base_interoperability_module.ts#L149-L152 use
BigInt(0)
for comparison
Please include it in the same PR
framework/test/unit/modules/interoperability/mainchain/module.spec.ts
Outdated
Show resolved
Hide resolved
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.
Overall looks fine to me,
One issue, the comparison should be ownChainNonce <= BigInt(0)
https://github.com/LiskHQ/lisk-sdk/blob/24cdc25d44b35c9c406079c7c8d795bb571bd5e6/framework/src/modules/interoperability/mainchain/module.ts#L268C41-L268C54
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.
Very nice, good job! Approved, with an optional small improvement suggestion.
framework/src/modules/interoperability/base_interoperability_module.ts
Outdated
Show resolved
Hide resolved
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.
Looks good to me!
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.
Good work @sitetester 👏
…ommon-is-called-inside-loop
…de-loop' of github.com:LiskHQ/lisk-sdk into 8998-verifyTerminatedStateAccountsCommon-is-called-inside-loop
What was the problem?
This PR resolves #8998
How was it solved?
Bug fixed & genesis block updated
Steps to update genesis block:
Debugging tips:
rm -rf ~/.lisk/pos-mainchain-fast
How was it tested?
Relevant test added