-
Notifications
You must be signed in to change notification settings - Fork 345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add pagination to sensitive endpoints #1601
feat: add pagination to sensitive endpoints #1601
Conversation
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 job! left just a few minor comments
// GetPendingPacketsByAddressPaginated retrieves rollapp packets from the KVStore by their receiver with pagination. | ||
func (k Keeper) GetPendingPacketsByAddressPaginated(ctx sdk.Context, receiver string, pageReq *query.PageRequest) (packets []commontypes.RollappPacket, pageResp *query.PageResponse, err error) { | ||
_, pageResp, err = collcompat.CollectionPaginate(ctx, k.pendingPacketsByAddress, pageReq, | ||
func(key collections.Pair[string, []byte], _ collections.NoValue) (bool, error) { | ||
packet, err := k.GetRollappPacket(ctx, string(key.K2())) | ||
if err != nil { | ||
return true, err | ||
} | ||
packets = append(packets, *packet) | ||
return false, nil | ||
}, collcompat.WithCollectionPaginationPairPrefix[string, []byte](receiver), | ||
) | ||
if err != nil { | ||
return nil, nil, err | ||
} | ||
|
||
return packets, pageResp, nil | ||
} |
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.
this could be simpler. you can directly return commontypes.RollappPacket
in transformFunc
while paginating.
// GetPendingPacketsByAddressPaginated retrieves rollapp packets from the KVStore by their receiver with pagination. | |
func (k Keeper) GetPendingPacketsByAddressPaginated(ctx sdk.Context, receiver string, pageReq *query.PageRequest) (packets []commontypes.RollappPacket, pageResp *query.PageResponse, err error) { | |
_, pageResp, err = collcompat.CollectionPaginate(ctx, k.pendingPacketsByAddress, pageReq, | |
func(key collections.Pair[string, []byte], _ collections.NoValue) (bool, error) { | |
packet, err := k.GetRollappPacket(ctx, string(key.K2())) | |
if err != nil { | |
return true, err | |
} | |
packets = append(packets, *packet) | |
return false, nil | |
}, collcompat.WithCollectionPaginationPairPrefix[string, []byte](receiver), | |
) | |
if err != nil { | |
return nil, nil, err | |
} | |
return packets, pageResp, nil | |
} | |
// GetPendingPacketsByAddressPaginated retrieves rollapp packets from the KVStore by their receiver with pagination. | |
func (k Keeper) GetPendingPacketsByAddressPaginated(ctx sdk.Context, receiver string, pageReq *query.PageRequest) ([]commontypes.RollappPacket, *query.PageResponse, error) { | |
return collcompat.CollectionPaginate(ctx, k.pendingPacketsByAddress, pageReq, | |
func(key collections.Pair[string, []byte], _ collections.NoValue) (commontypes.RollappPacket, error) { | |
packet, err := k.GetRollappPacket(ctx, string(key.K2())) | |
return *packet, err | |
}, collcompat.WithCollectionPaginationPairPrefix[string, []byte](receiver), | |
) | |
} |
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.
packet, err := k.GetRollappPacket(ctx, string(key.K2()))
return *packet, err
packet
could be nil though
func (k Keeper) GetAllRegisteredDenomsPaginated(ctx sdk.Context, rollappID string, pageReq *query.PageRequest) ([]string, *query.PageResponse, error) { | ||
denoms, pageRes, err := collcompat.CollectionPaginate(ctx, k.registeredRollappDenoms, pageReq, | ||
func(key collections.Pair[string, string], _ collections.NoValue) (string, error) { | ||
return key.K1(), nil | ||
}, collcompat.WithCollectionPaginationPairPrefix[string, string](rollappID), | ||
) | ||
if err != nil { | ||
return nil, nil, fmt.Errorf("paginate registered denoms: %w", err) | ||
} | ||
|
||
return denoms, pageRes, nil | ||
} |
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.
i think you can skip the error check here
func (k Keeper) GetAllRegisteredDenomsPaginated(ctx sdk.Context, rollappID string, pageReq *query.PageRequest) ([]string, *query.PageResponse, error) { | |
denoms, pageRes, err := collcompat.CollectionPaginate(ctx, k.registeredRollappDenoms, pageReq, | |
func(key collections.Pair[string, string], _ collections.NoValue) (string, error) { | |
return key.K1(), nil | |
}, collcompat.WithCollectionPaginationPairPrefix[string, string](rollappID), | |
) | |
if err != nil { | |
return nil, nil, fmt.Errorf("paginate registered denoms: %w", err) | |
} | |
return denoms, pageRes, nil | |
} | |
func (k Keeper) GetAllRegisteredDenomsPaginated(ctx sdk.Context, rollappID string, pageReq *query.PageRequest) ([]string, *query.PageResponse, error) { | |
return collcompat.CollectionPaginate(ctx, k.registeredRollappDenoms, pageReq, | |
func(key collections.Pair[string, string], _ collections.NoValue) (string, error) { | |
return key.K1(), nil | |
}, collcompat.WithCollectionPaginationPairPrefix[string, string](rollappID), | |
) | |
} |
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.
why didn't I think of that
also, tests and linter are failing |
Description
Closes #1591
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...
Unreleased
section inCHANGELOG.md
godoc
commentsSDK Checklist
map
time.Now()
sendCoin
and notSendCoins
Full security checklist here
For Reviewer:
After reviewer approval: