Fix: Iteration through PacketMaturityTimes assumes maturity time order #622
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ package keeper | |
| import ( | ||
| "encoding/binary" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "github.com/cosmos/cosmos-sdk/codec" | ||
| sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
|
|
@@ -206,76 +207,81 @@ func (k Keeper) DeletePendingChanges(ctx sdk.Context) { | |
| store.Delete(types.PendingChangesKey()) | ||
| } | ||
|
|
||
| // GetElapsedPacketMaturityTimes returns a slice of already elapsed PacketMaturityTimes, sorted by maturity times, | ||
| // i.e., the slice contains the IDs of the matured VSCPackets. | ||
| func (k Keeper) GetElapsedPacketMaturityTimes(ctx sdk.Context) (maturingVSCPackets []consumertypes.MaturingVSCPacket) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| iterator := sdk.KVStorePrefixIterator(store, []byte{types.PacketMaturityTimeBytePrefix}) | ||
|
|
||
| defer iterator.Close() | ||
|
|
||
| for ; iterator.Valid(); iterator.Next() { | ||
| var maturingVSCPacket consumertypes.MaturingVSCPacket | ||
| if err := maturingVSCPacket.Unmarshal(iterator.Value()); err != nil { | ||
| panic(fmt.Errorf("failed to unmarshal MaturingVSCPacket: %w", err)) | ||
| } | ||
|
|
||
| // If the maturity time is after the current time, then stop the iteration; | ||
mpoke marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // this is possible since the iteration over PacketMaturityTimes is in order | ||
| // of maturity times | ||
| if ctx.BlockTime().Before(maturingVSCPacket.MaturityTime) { | ||
| break | ||
| } | ||
|
|
||
| maturingVSCPackets = append(maturingVSCPackets, maturingVSCPacket) | ||
| } | ||
| return maturingVSCPackets | ||
| } | ||
|
|
||
| // GetAllPacketMaturityTimes returns a slice of all PacketMaturityTimes, sorted by maturity times. | ||
| // | ||
| // Note that PacketMaturityTimes are stored under keys with the following format: | ||
| // PacketMaturityTimeBytePrefix | maturityTime.UnixNano() | vscID | ||
| // Thus, the returned array is in ascending order of maturityTimes. | ||
| // If two entries have the same maturityTime, then they are ordered by vscID. | ||
| func (k Keeper) GetAllPacketMaturityTimes(ctx sdk.Context) (maturingVSCPackets []consumertypes.MaturingVSCPacket) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| iterator := sdk.KVStorePrefixIterator(store, []byte{types.PacketMaturityTimeBytePrefix}) | ||
|
|
||
| defer iterator.Close() | ||
| for ; iterator.Valid(); iterator.Next() { | ||
| var maturingVSCPacket consumertypes.MaturingVSCPacket | ||
| if err := maturingVSCPacket.Unmarshal(iterator.Value()); err != nil { | ||
| panic(fmt.Errorf("failed to unmarshal MaturingVSCPacket: %w", err)) | ||
| } | ||
|
|
||
| maturingVSCPackets = append(maturingVSCPackets, maturingVSCPacket) | ||
| } | ||
| return maturingVSCPackets | ||
| } | ||
|
|
||
| // SetPacketMaturityTime sets the maturity time for a given received VSC packet id | ||
| func (k Keeper) SetPacketMaturityTime(ctx sdk.Context, vscId uint64, maturityTime time.Time) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| maturingVSCPacket := consumertypes.MaturingVSCPacket{ | ||
| VscId: vscId, | ||
| MaturityTime: maturityTime, | ||
| } | ||
| bz, err := maturingVSCPacket.Marshal() | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to marshal MaturingVSCPacket: %w", err)) | ||
| } | ||
| store.Set(types.PacketMaturityTimeKey(vscId, maturityTime), bz) | ||
| } | ||
|
|
||
|
There was a problem hiding this comment.
@MSalopek @jtremback this means it would be hard to query the maturity of a VSCPacket (as we don't know the maturity time). |
||
| // PacketMaturityExists checks whether the packet maturity time for a given vscId and maturityTime exists. | ||
| // | ||
| // Note: this method is only used in testing. | ||
| func (k Keeper) PacketMaturityTimeExists(ctx sdk.Context, vscId uint64, maturityTime time.Time) bool { | ||
| store := ctx.KVStore(k.storeKey) | ||
| bz := store.Get(types.PacketMaturityTimeKey(vscId, maturityTime)) | ||
| return bz != nil | ||
shaspitz marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // DeletePacketMaturityTimes deletes the packet maturity time for a given vscId and maturityTime | ||
| func (k Keeper) DeletePacketMaturityTimes(ctx sdk.Context, vscId uint64, maturityTime time.Time) { | ||
| store := ctx.KVStore(k.storeKey) | ||
| store.Delete(types.PacketMaturityTimeKey(vscId, maturityTime)) | ||
| } | ||
|
|
||
| // VerifyProviderChain verifies that the chain trying to connect on the channel handshake | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Prob worth noting that if this time is being stored on the consumer at all, the maturity time is in the future. Otherwise a VSCMatured packet is sent from consumer -> provider.
Is my understanding correct?
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.
Almost. This is set either in
InitGenesison a consumer restart in which case the maturity time could be in the past, which would result inVSCMaturedPacketssent in the firstEndBlock.Most likely though, the maturity time is set in
OnRecvVSCPacket, i.e.,