feat(delayedack): delayedack invariant for finalized and reverted packets #686
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #686 +/- ##
==========================================
+ Coverage 30.69% 31.37% +0.68%
==========================================
Files 227 234 +7
Lines 32052 32661 +609
==========================================
+ Hits 9837 10248 +411
- Misses 20653 20781 +128
- Partials 1562 1632 +70 ☔ View full report in Codecov by Sentry. |
omritoptix
changed the title
srene
changed the title
srene
force-pushed
the
srene/672-finalized-rollapp-packets-invariant
branch
from
5338daa
to
106dfd1
Compare
Co-authored-by: Daniel T <[email protected]>
| ProofHeight: uint64(rollappBlocks[rollapp] + k), | ||
| } |
There was a problem hiding this comment.
is this proof height right?
If it is, please comment above, it's hard to understand
| // send state updates | ||
| var lastHeight uint64 = 0 | ||
|
|
||
| sequence := uint64(0) |
There was a problem hiding this comment.
I guess you could move this inside the rollapp-wise loop? Maybe it doesn't matter
There was a problem hiding this comment.
sequence should be per packet, so i think is ok outside
| lastHeight = lastHeight + numOfBlocks | ||
| } |
There was a problem hiding this comment.
it's a little confusing to have a last height across all rollapps, rather than rollapp-wise
There was a problem hiding this comment.
lastheight has been removed because it was not really necessary
omritoptix
changed the title
|
|
||
| func (suite *DelayedAckTestSuite) TestInvariants() { | ||
| suite.SetupTest() | ||
| initialheight := int64(10) |
| suite.Require().NoError(err) | ||
| for k := uint64(1); k <= numOfBlocks; k++ { | ||
| // calculating a different proof height incrementing a block height for each new packet | ||
| proofheight := rollappBlocks[rollapp] + k |
| msg, bool := keeper.AllInvariants(suite.App.DelayedAckKeeper)(suite.Ctx) | ||
| suite.Require().False(bool, msg) |
There was a problem hiding this comment.
| msg, bool := keeper.AllInvariants(suite.App.DelayedAckKeeper)(suite.Ctx) | |
| suite.Require().False(bool, msg) | |
| msg, fails := keeper.AllInvariants(suite.App.DelayedAckKeeper)(suite.Ctx) | |
| suite.Require().False(bool, fails) |
bool collides
There was a problem hiding this comment.
i guess you meant to replace bool by fails
Description
This PRs adds a new invariant that checks all IBC rollapp packets stored for the delayedack module for a finalized height are also in finalized status
Closes #672
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleasedsection inCHANGELOG.mdgodoccommentsSDK Checklist
maptime.Now()sendCoinand notSendCoinsFull security checklist here
----;
For Reviewer:
---;
After reviewer approval: