Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions proto/canine_chain/storage/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@ service Query {
"/jackal/canine-chain/storage/files/merkle/{merkle}";
}

// Queries a list of File items owned by a specific address.
rpc AllFilesByOwner(QueryAllFilesByOwner)
returns (QueryAllFilesByOwnerResponse) {
option (google.api.http).get =
"/jackal/canine-chain/storage/files/owner/{owner}";
}

// Queries a Proof by provider_address, merkle, owner, and start.
rpc Proof(QueryProof)
returns (QueryProofResponse) {
Expand Down
160 changes: 81 additions & 79 deletions x/storage/keeper/grpc_query_active_deals.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,58 @@ import (
"google.golang.org/grpc/status"
)

// paginationParams holds extracted pagination parameters
type paginationParams struct {
reverse bool
limit uint64
offset uint64
}

// extractPaginationParams extracts pagination parameters with defaults
func extractPaginationParams(pagination *query.PageRequest) paginationParams {
p := paginationParams{
reverse: false,
limit: 100,
offset: 0,
}
if pagination != nil {
p.reverse = pagination.Reverse
if pagination.Limit > 0 {
p.limit = pagination.Limit
}
p.offset = pagination.Offset
}
return p
}

// filterFilesWithPagination iterates over files and returns those matching the filter with pagination
func (k Keeper) filterFilesWithPagination(ctx sdk.Context, params paginationParams, filter func(*types.UnifiedFile) bool) ([]types.UnifiedFile, *query.PageResponse) {
var files []types.UnifiedFile
var skipped, collected, total uint64

k.IterateFilesByMerkle(ctx, params.reverse, func(_ []byte, val []byte) bool {
var file types.UnifiedFile
if err := k.cdc.Unmarshal(val, &file); err != nil {
return false
}
Comment on lines +44 to +48

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃Ч Nitpick | 馃數 Trivial

Consider logging unmarshal errors instead of silently skipping.

When Unmarshal fails, the iteration silently continues. This provides resilience but could mask data corruption issues. Consider adding a log statement to aid debugging.

馃摑 Optional: Add error logging
 	k.IterateFilesByMerkle(ctx, params.reverse, func(_ []byte, val []byte) bool {
 		var file types.UnifiedFile
 		if err := k.cdc.Unmarshal(val, &file); err != nil {
+			k.Logger(ctx).Error("failed to unmarshal file during pagination", "error", err)
 			return false
 		}
馃摑 Committable suggestion

鈥硷笍 IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
k.IterateFilesByMerkle(ctx, params.reverse, func(_ []byte, val []byte) bool {
var file types.UnifiedFile
if err := k.cdc.Unmarshal(val, &file); err != nil {
return false
}
k.IterateFilesByMerkle(ctx, params.reverse, func(_ []byte, val []byte) bool {
var file types.UnifiedFile
if err := k.cdc.Unmarshal(val, &file); err != nil {
k.Logger(ctx).Error("failed to unmarshal file during pagination", "error", err)
return false
}
馃 Prompt for AI Agents
In `@x/storage/keeper/grpc_query_active_deals.go` around lines 44 - 48, The
iteration in k.IterateFilesByMerkle silently skips entries when
k.cdc.Unmarshal(val, &file) fails; change this to log the unmarshal error before
returning false so data issues are visible鈥攗se the keeper's logger (or context
logger) to emit a descriptive message including the error and any identifying
bytes (e.g., the val or merkle key) in the k.IterateFilesByMerkle callback when
k.cdc.Unmarshal returns an error for types.UnifiedFile.


if filter(&file) {
total++
if skipped < params.offset {
skipped++
return false
}
if collected < params.limit {
files = append(files, file)
collected++
}
}
return false
})

return files, &query.PageResponse{Total: total}
}

func (k Keeper) AllFiles(c context.Context, req *types.QueryAllFiles) (*types.QueryAllFilesResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
Expand Down Expand Up @@ -110,110 +162,60 @@ func (k Keeper) AllFilesByMerkle(c context.Context, req *types.QueryAllFilesByMe
return &types.QueryAllFilesByMerkleResponse{Files: files, Pagination: pageRes}, nil
}

// OpenFiles returns a paginated list of files with space that providers have yet to fill
//
// TODO: Create unit-test cases for this
func (k Keeper) OpenFiles(c context.Context, req *types.QueryOpenFiles) (*types.QueryAllFilesResponse, error) {
// AllFilesByOwner returns a paginated list of files owned by a specific address
func (k Keeper) AllFilesByOwner(c context.Context, req *types.QueryAllFilesByOwner) (*types.QueryAllFilesByOwnerResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}

var files []types.UnifiedFile
ctx := sdk.UnwrapSDKContext(c)
if req.Owner == "" {
return nil, status.Error(codes.InvalidArgument, "owner address is required")
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

reverse := false
var limit uint64 = 100
if req.Pagination != nil { // HERE IS THE FIX
reverse = req.Pagination.Reverse
limit = req.Pagination.Limit
if _, err := sdk.AccAddressFromBech32(req.Owner); err != nil {
return nil, status.Error(codes.InvalidArgument, "invalid owner address format")
}

var i uint64
var total uint64
k.IterateFilesByMerkle(ctx, reverse, func(_ []byte, val []byte) bool {
var file types.UnifiedFile
if err := k.cdc.Unmarshal(val, &file); err != nil {
return false
}
ctx := sdk.UnwrapSDKContext(c)
params := extractPaginationParams(req.Pagination)

if file.ContainsProver(req.ProviderAddress) {
return false
}
files, pageRes := k.filterFilesWithPagination(ctx, params, func(file *types.UnifiedFile) bool {
return file.Owner == req.Owner
})

if len(file.Proofs) < int(file.MaxProofs) {
total++
if i >= limit {
return false
}
files = append(files, file)
} else {
return false
}
return &types.QueryAllFilesByOwnerResponse{Files: files, Pagination: pageRes}, nil
}

i++
// OpenFiles returns a paginated list of files with space that providers have yet to fill
func (k Keeper) OpenFiles(c context.Context, req *types.QueryOpenFiles) (*types.QueryAllFilesResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}

return false
})
ctx := sdk.UnwrapSDKContext(c)
params := extractPaginationParams(req.Pagination)

qpr := query.PageResponse{
NextKey: nil,
Total: total,
}
files, pageRes := k.filterFilesWithPagination(ctx, params, func(file *types.UnifiedFile) bool {
return !file.ContainsProver(req.ProviderAddress) && len(file.Proofs) < int(file.MaxProofs)
})

return &types.QueryAllFilesResponse{Files: files, Pagination: &qpr}, nil
return &types.QueryAllFilesResponse{Files: files, Pagination: pageRes}, nil
}

// EndangeredFiles returns a paginated list of files with only 1x redundancy
//
// TODO: Create unit-test cases for this
func (k Keeper) EndangeredFiles(c context.Context, req *types.QueryOpenFiles) (*types.QueryAllFilesResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "invalid request")
}

var files []types.UnifiedFile
ctx := sdk.UnwrapSDKContext(c)
params := extractPaginationParams(req.Pagination)

reverse := false
var limit uint64 = 100
if req.Pagination != nil { // HERE IS THE FIX
reverse = req.Pagination.Reverse
limit = req.Pagination.Limit
}

var i uint64
var total uint64
k.IterateFilesByMerkle(ctx, reverse, func(_ []byte, val []byte) bool {
var file types.UnifiedFile
if err := k.cdc.Unmarshal(val, &file); err != nil {
return false
}

if file.ContainsProver(req.ProviderAddress) {
return false
}

if len(file.Proofs) == 1 {
total++
if i >= limit {
return false
}
files = append(files, file)
} else {
return false
}

i++

return false
files, pageRes := k.filterFilesWithPagination(ctx, params, func(file *types.UnifiedFile) bool {
return !file.ContainsProver(req.ProviderAddress) && len(file.Proofs) == 1
})

qpr := query.PageResponse{
NextKey: nil,
Total: total,
}

return &types.QueryAllFilesResponse{Files: files, Pagination: &qpr}, nil
return &types.QueryAllFilesResponse{Files: files, Pagination: pageRes}, nil
}

func (k Keeper) File(c context.Context, req *types.QueryFile) (*types.QueryFileResponse, error) {
Expand Down
153 changes: 153 additions & 0 deletions x/storage/keeper/grpc_query_active_deals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,159 @@ func (suite *KeeperTestSuite) TestAllFiles() {
suite.reset()
}

func (suite *KeeperTestSuite) TestAllFilesByOwner() {
suite.SetupSuite()

testAddresses, err := testutil.CreateTestAddresses("cosmos", 3)
suite.Require().NoError(err)

owner1 := testAddresses[0]
owner2 := testAddresses[1]
depoAccount := testAddresses[2]

coins := sdk.NewCoins(sdk.NewCoin("ujkl", sdk.NewInt(100000000000)))
testAcc, _ := sdk.AccAddressFromBech32(owner1)
err = suite.bankKeeper.SendCoinsFromModuleToAccount(suite.ctx, types.ModuleName, testAcc, coins)
suite.Require().NoError(err)

suite.storageKeeper.SetParams(suite.ctx, types.Params{
DepositAccount: depoAccount,
ProofWindow: 50,
ChunkSize: 1024,
PriceFeed: "jklprice",
MissesToBurn: 3,
MaxContractAgeInBlocks: 100,
PricePerTbPerMonth: 8,
CollateralPrice: 2,
CheckWindow: 11,
ReferralCommission: 25,
PolRatio: 40,
})

// Create 5 files for owner1
for i := 0; i < 5; i++ {
merkle := []byte(fmt.Sprintf("merkle_owner1_%d", i))
suite.storageKeeper.SetFile(suite.ctx, types.UnifiedFile{
Merkle: merkle,
Owner: owner1,
Start: int64(i),
Expires: 0,
FileSize: 1024,
ProofInterval: 400,
ProofType: 0,
Proofs: make([]string, 0),
MaxProofs: 3,
Note: "{}",
})
}

// Create 3 files for owner2
for i := 0; i < 3; i++ {
merkle := []byte(fmt.Sprintf("merkle_owner2_%d", i))
suite.storageKeeper.SetFile(suite.ctx, types.UnifiedFile{
Merkle: merkle,
Owner: owner2,
Start: int64(i),
Expires: 0,
FileSize: 2048,
ProofInterval: 400,
ProofType: 0,
Proofs: make([]string, 0),
MaxProofs: 3,
Note: "{}",
})
}

// Test: Query files for owner1 - should return 5 files
pg := query.PageRequest{
Offset: 0,
Reverse: false,
Limit: 100,
}

res, err := suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
Pagination: &pg,
Owner: owner1,
})
suite.Require().NoError(err)
suite.Require().Equal(5, len(res.Files))
suite.Require().Equal(uint64(5), res.Pagination.Total)

// Verify all returned files belong to owner1
for _, file := range res.Files {
suite.Require().Equal(owner1, file.Owner)
}

// Test: Query files for owner2 - should return 3 files
res, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
Pagination: &pg,
Owner: owner2,
})
suite.Require().NoError(err)
suite.Require().Equal(3, len(res.Files))
suite.Require().Equal(uint64(3), res.Pagination.Total)

// Verify all returned files belong to owner2
for _, file := range res.Files {
suite.Require().Equal(owner2, file.Owner)
}

// Test: Pagination with limit
pgLimit := query.PageRequest{
Offset: 0,
Limit: 2,
}
res, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
Pagination: &pgLimit,
Owner: owner1,
})
suite.Require().NoError(err)
suite.Require().Equal(2, len(res.Files))
suite.Require().Equal(uint64(5), res.Pagination.Total) // Total should still be 5

// Test: Pagination with offset
pgOffset := query.PageRequest{
Offset: 3,
Limit: 100,
}
res, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
Pagination: &pgOffset,
Owner: owner1,
})
suite.Require().NoError(err)
suite.Require().Equal(2, len(res.Files)) // 5 total - 3 offset = 2 remaining
suite.Require().Equal(uint64(5), res.Pagination.Total)

// Test: Empty owner should return error
_, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
Pagination: &pg,
Owner: "",
})
suite.Require().Error(err)
suite.Require().Contains(err.Error(), "owner address is required")

// Test: Invalid owner address format should return error
_, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
Pagination: &pg,
Owner: "invalid-address",
})
suite.Require().Error(err)
suite.Require().Contains(err.Error(), "invalid owner address format")

// Test: Non-existent owner should return empty list
// Use a known valid bech32 address that won't match any files we created
nonExistentOwner := "cosmos1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqnrql8a"
res, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
Pagination: &pg,
Owner: nonExistentOwner,
})
suite.Require().NoError(err)
suite.Require().Equal(0, len(res.Files))
suite.Require().Equal(uint64(0), res.Pagination.Total)

suite.reset()
}
Comment on lines +147 to +298

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃Ч Nitpick | 馃數 Trivial

Well-structured test with comprehensive coverage.

The test thoroughly covers the AllFilesByOwner query endpoint including:

  • Basic owner filtering with multiple owners
  • Pagination behavior (limit and offset)
  • Error cases (empty owner, invalid format)
  • Non-existent owner edge case

The test follows existing patterns in the file and properly verifies that Pagination.Total reflects the filtered count rather than page size.

Minor suggestion: Consider adding a test case for cursor-based pagination using NextKey to ensure the query handler supports both pagination styles consistently with other endpoints like OpenFiles.

馃摑 Optional: Add cursor-based pagination test
 	suite.Require().Equal(2, len(res.Files)) // 5 total - 3 offset = 2 remaining
 	suite.Require().Equal(uint64(5), res.Pagination.Total)

+	// Test: Cursor-based pagination using NextKey
+	pgCursor := query.PageRequest{
+		Limit:      2,
+		CountTotal: true,
+	}
+	res, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
+		Pagination: &pgCursor,
+		Owner:      owner1,
+	})
+	suite.Require().NoError(err)
+	suite.Require().Equal(2, len(res.Files))
+	suite.Require().NotNil(res.Pagination.NextKey) // Should have more pages
+
+	// Fetch next page using NextKey
+	pgCursor.Key = res.Pagination.NextKey
+	res, err = suite.queryClient.AllFilesByOwner(context.Background(), &types.QueryAllFilesByOwner{
+		Pagination: &pgCursor,
+		Owner:      owner1,
+	})
+	suite.Require().NoError(err)
+	suite.Require().Equal(2, len(res.Files))
+
 	// Test: Empty owner should return error
馃 Prompt for AI Agents
In `@x/storage/keeper/grpc_query_active_deals_test.go` around lines 147 - 298, Add
a cursor-based pagination subtest inside TestAllFilesByOwner that mirrors the
existing limit/offset checks but uses query.PageRequest.NextKey to paginate:
call suite.queryClient.AllFilesByOwner with a first request that sets Limit
(e.g., 2) and nil Offset, capture res.Pagination.NextKey, then issue a second
request with Pagination.NextKey set to that value and verify combined results
equal the full set and res.Pagination.Total remains the same; reference the
TestAllFilesByOwner test, the suite.queryClient.AllFilesByOwner call,
types.QueryAllFilesByOwner, and query.PageRequest.NextKey/Limit to locate where
to add this check.


func (suite *KeeperTestSuite) TestOpenFiles() {
suite.SetupSuite()

Expand Down
Loading
Loading