feat(eibc): Expand list-demand-orders query with more parameters #851
Conversation
zale144
force-pushed
the
zale144/849-list-demand-orders-filters
branch
from
43fc95c
to
1e4569d
Compare
zale144
changed the base branch from
main
to
zale144/850-query-delayedack-by-type
zale144
changed the title
omritoptix
changed the title
| if err != nil { | ||
| return fmt.Errorf("limit must be an integer: %s", args[3]) | ||
| } | ||
| request.Limit = int32(limit) |
There was a problem hiding this comment.
Any reason why we use limit instead of Pagination in cosmos-sdk?
There was a problem hiding this comment.
well, the intent of this added filtering is just for debugging, it's not supposed to be for end users
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you! |
| @@ -26,6 +26,7 @@ func PendingByRollappIDByMaxHeight( | |||
| End: commontypes.RollappPacketByStatusByRollappIDByProofHeightPrefix(rollappID, status, maxProofHeight+1), // inclusive end | |||
| }, | |||
| }, | |||
| FilterFunc: func(packet commontypes.RollappPacket) bool { return true }, | |||
There was a problem hiding this comment.
should define it (func(packet commontypes.RollappPacket) bool { return true } )and reuse for readability
…nd-orders-filters
…list-demand-orders-filters
…list-demand-orders-filters
There was a problem hiding this comment.
state breaking? will require migration
| @@ -177,6 +178,11 @@ func (k Keeper) ListDemandOrdersByStatus(ctx sdk.Context, status commontypes.Sta | |||
| for ; iterator.Valid(); iterator.Next() { | |||
| var val types.DemandOrder | |||
| k.cdc.MustUnmarshal(iterator.Value(), &val) | |||
| for _, opt := range opts { | |||
There was a problem hiding this comment.
the limit(limit int) option is bit miss the point here no?
u'll still process and unmarshal the entire query
I think better design for limit should break this iterator
Description
Closes #849
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: