From d1b96feebb3f28914b5791bea1728f798fd03a01 Mon Sep 17 00:00:00 2001 From: J Bilal rafique Date: Fri, 7 Nov 2025 15:36:46 +0000 Subject: [PATCH] Centralize rounding functions and tests --- go.sum | 2 - sdk/action/client.go | 6 +- supernode/cascade/helper.go | 5 +- supernode/cascade/helper_test.go | 253 +++++++++++++++++++++++++++++++ 4 files changed, 257 insertions(+), 9 deletions(-) create mode 100644 supernode/cascade/helper_test.go diff --git a/go.sum b/go.sum index 2ce74b3b..1ebad53e 100644 --- a/go.sum +++ b/go.sum @@ -76,8 +76,6 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.50 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.50.0 h1:ig/FpDD2JofP/NExKQUbn7uOSZzJAQqogfqluZK4ed4= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.50.0/go.mod h1:otE2jQekW/PqXk1Awf5lmfokJx4uwuqcj1ab5SpGeW0= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= -github.com/LumeraProtocol/lumera v1.8.4 h1:6XzLS9gd0m3lOnppNS05WuZx4VCBEGvUN/KpVkSjqro= -github.com/LumeraProtocol/lumera v1.8.4/go.mod h1:twrSLfuXcHvmfQoN5e02Bg7rfeevUjF34SVqEJIvH1E= github.com/LumeraProtocol/rq-go v0.2.1 h1:8B3UzRChLsGMmvZ+UVbJsJj6JZzL9P9iYxbdUwGsQI4= github.com/LumeraProtocol/rq-go v0.2.1/go.mod h1:APnKCZRh1Es2Vtrd2w4kCLgAyaL5Bqrkz/BURoRJ+O8= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= diff --git a/sdk/action/client.go b/sdk/action/client.go index 81aa806b..e6e85f65 100644 --- a/sdk/action/client.go +++ b/sdk/action/client.go @@ -298,11 +298,9 @@ func (c *ClientImpl) BuildCascadeMetadataFromFile(ctx context.Context, filePath denom := paramsResp.Params.BaseActionFee.Denom exp := paramsResp.Params.ExpirationDuration - // Compute data size in KB for fee, rounding up to avoid underpaying - // Keep consistent with supernode verification which uses ceil(bytes/1024) sizeBytes := fi.Size() - kb := (sizeBytes + 1023) / 1024 // int64 division - feeResp, err := c.lumeraClient.GetActionFee(ctx, strconv.FormatInt(kb, 10)) + kb := actiontypes.RoundBytesToKB64(sizeBytes) + feeResp, err := c.lumeraClient.GetActionFee(ctx, strconv.Itoa(kb)) if err != nil { return actiontypes.CascadeMetadata{}, "", "", fmt.Errorf("get action fee: %w", err) } diff --git a/supernode/cascade/helper.go b/supernode/cascade/helper.go index 394b59f1..4eb41c77 100644 --- a/supernode/cascade/helper.go +++ b/supernode/cascade/helper.go @@ -180,8 +180,7 @@ func (task *CascadeRegistrationTask) verifyActionFee(ctx context.Context, action } fields["data_bytes"] = dataSize logtrace.Info(ctx, "register: verify action fee start", fields) - // Round up to the nearest KB to avoid underestimating required fee - dataSizeInKBs := (dataSize + 1023) / 1024 + dataSizeInKBs := actiontypes.RoundBytesToKB(dataSize) fee, err := task.LumeraClient.GetActionFee(ctx, strconv.Itoa(dataSizeInKBs)) if err != nil { return task.wrapErr(ctx, "failed to get action fee", err, fields) @@ -198,7 +197,7 @@ func (task *CascadeRegistrationTask) verifyActionFee(ctx context.Context, action } providedFee, err := sdk.ParseCoinNormalized(action.Price) if err != nil { - return task.wrapErr(ctx, "invalid fee format", errors.Errorf("price parse error: %v", err), fields) + return task.wrapErr(ctx, "invalid fee format", errors.Errorf("failed to parse price '%s': %v", action.Price, err), fields) } if providedFee.Denom != requiredFee.Denom { return task.wrapErr(ctx, "invalid fee denom", errors.Errorf("expected denom %s, got %s", requiredFee.Denom, providedFee.Denom), fields) diff --git a/supernode/cascade/helper_test.go b/supernode/cascade/helper_test.go new file mode 100644 index 00000000..d6bdcb1d --- /dev/null +++ b/supernode/cascade/helper_test.go @@ -0,0 +1,253 @@ +package cascade + +import ( + "context" + "fmt" + "testing" + + "cosmossdk.io/math" + actiontypes "github.com/LumeraProtocol/lumera/x/action/v1/types" + sntypes "github.com/LumeraProtocol/lumera/x/supernode/v1/types" + "github.com/LumeraProtocol/supernode/v2/supernode/adaptors" + sdktx "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/stretchr/testify/assert" +) + +// mockLumeraClient implements the LumeraClient interface for testing +type mockLumeraClient struct { + baseActionFee math.Int + feePerKbyte math.Int +} + +// Compile-time check to ensure mockLumeraClient implements adaptors.LumeraClient +var _ adaptors.LumeraClient = (*mockLumeraClient)(nil) + +func newMockLumeraClient(baseFee, perKB int64) *mockLumeraClient { + return &mockLumeraClient{ + baseActionFee: math.NewInt(baseFee), + feePerKbyte: math.NewInt(perKB), + } +} + +func (m *mockLumeraClient) GetActionFee(ctx context.Context, dataSizeKB string) (*actiontypes.QueryGetActionFeeResponse, error) { + // Parse KB as int64 + var kb int64 + _, err := fmt.Sscanf(dataSizeKB, "%d", &kb) + if err != nil { + return nil, fmt.Errorf("invalid dataSizeKB: %s", dataSizeKB) + } + + // Fee calculation: (FeePerKbyte × KB) + BaseActionFee + perByteCost := m.feePerKbyte.MulRaw(kb) + totalAmount := perByteCost.Add(m.baseActionFee) + + return &actiontypes.QueryGetActionFeeResponse{ + Amount: totalAmount.String(), + }, nil +} + +func (m *mockLumeraClient) GetAction(ctx context.Context, actionID string) (*actiontypes.QueryGetActionResponse, error) { + return nil, fmt.Errorf("not implemented in mock") +} + +func (m *mockLumeraClient) GetTopSupernodes(ctx context.Context, blockHeight uint64) (*sntypes.QueryGetTopSuperNodesForBlockResponse, error) { + return nil, fmt.Errorf("not implemented in mock") +} + +func (m *mockLumeraClient) Verify(ctx context.Context, address string, msg []byte, sig []byte) error { + return fmt.Errorf("not implemented in mock") +} + +func (m *mockLumeraClient) SimulateFinalizeAction(ctx context.Context, actionID string, rqids []string) (*sdktx.SimulateResponse, error) { + return nil, fmt.Errorf("not implemented in mock") +} + +func (m *mockLumeraClient) FinalizeAction(ctx context.Context, actionID string, rqids []string) (*sdktx.BroadcastTxResponse, error) { + return nil, fmt.Errorf("not implemented in mock") +} + +// TestVerifyActionFee_EdgeCases tests the verifyActionFee function with edge cases +func TestVerifyActionFee_EdgeCases(t *testing.T) { + testCases := []struct { + name string + dataBytes int // Input data size in bytes + actionPrice string // Price set on the action + baseFee int64 // Base action fee param + feePerKB int64 // Fee per KB param + shouldPass bool // Should validation pass? + description string + }{ + { + name: "0 bytes - exact fee should pass", + dataBytes: 0, + actionPrice: "10000ulume", // 0 KB → fee = 10000 + baseFee: 10000, + feePerKB: 10, + shouldPass: true, + description: "0 bytes rounds to 0 KB, fee = base only", + }, + { + name: "0 bytes - underpayment should fail", + dataBytes: 0, + actionPrice: "9999ulume", // Less than required 10000 + baseFee: 10000, + feePerKB: 10, + shouldPass: false, + description: "Paying less than base fee should fail", + }, + { + name: "1 byte - exact fee should pass", + dataBytes: 1, + actionPrice: "10010ulume", // 1 byte → 1 KB → fee = 10010 + baseFee: 10000, + feePerKB: 10, + shouldPass: true, + description: "1 byte rounds to 1 KB, fee = 10 + 10000", + }, + { + name: "1 byte - underpayment should fail", + dataBytes: 1, + actionPrice: "10009ulume", // Less than required 10010 + baseFee: 10000, + feePerKB: 10, + shouldPass: false, + description: "Paying less than required fee should fail", + }, + { + name: "1025 bytes (1 chunk + 1 byte) - exact fee should pass", + dataBytes: 1025, + actionPrice: "10020ulume", // 1025 bytes → 2 KB → fee = 20 + 10000 + baseFee: 10000, + feePerKB: 10, + shouldPass: true, + description: "1025 bytes rounds to 2 KB, fee = 20 + 10000", + }, + { + name: "1025 bytes - underpayment should fail", + dataBytes: 1025, + actionPrice: "10019ulume", // Less than required 10020 + baseFee: 10000, + feePerKB: 10, + shouldPass: false, + description: "Paying less than 2 KB worth should fail", + }, + { + name: "1024 bytes (exactly 1 KB) - exact fee should pass", + dataBytes: 1024, + actionPrice: "10010ulume", // 1024 bytes → 1 KB → fee = 10 + 10000 + baseFee: 10000, + feePerKB: 10, + shouldPass: true, + description: "Exactly 1 KB should charge for 1 KB", + }, + { + name: "overpayment is allowed", + dataBytes: 1, + actionPrice: "50000ulume", // Much more than required 10010 + baseFee: 10000, + feePerKB: 10, + shouldPass: true, + description: "Users can pay more than the minimum", + }, + { + name: "wrong denom should fail", + dataBytes: 1, + actionPrice: "10010usdt", // Wrong denom + baseFee: 10000, + feePerKB: 10, + shouldPass: false, + description: "Only ulume denom should be accepted", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + + // Create mock Lumera client with test params + mockClient := newMockLumeraClient(tc.baseFee, tc.feePerKB) + + // Create task with mock client + task := &CascadeRegistrationTask{ + CascadeService: &CascadeService{ + LumeraClient: mockClient, + }, + } + + // Create action with the price (stored as string in proto) + action := &actiontypes.Action{ + Price: tc.actionPrice, + } + + // Call verifyActionFee + err := task.verifyActionFee(ctx, action, tc.dataBytes, nil) + + // Check result + if tc.shouldPass { + assert.NoError(t, err, "Expected validation to pass but got error: %v\nDescription: %s", err, tc.description) + } else { + assert.Error(t, err, "Expected validation to fail\nDescription: %s", tc.description) + } + + t.Logf("Test: %s\n Data: %d bytes\n Price: %s\n Result: %v\n Description: %s", + tc.name, tc.dataBytes, tc.actionPrice, err, tc.description) + }) + } +} + +// TestVerifyActionFee_NilAction tests error handling when action.Price is empty +func TestVerifyActionFee_NilAction(t *testing.T) { + ctx := context.Background() + + mockClient := newMockLumeraClient(10000, 10) + task := &CascadeRegistrationTask{ + CascadeService: &CascadeService{ + LumeraClient: mockClient, + }, + } + + // Action with empty price (string) + action := &actiontypes.Action{ + Price: "", + } + + err := task.verifyActionFee(ctx, action, 100, nil) + assert.Error(t, err, "Should fail when action.Price is empty") + assert.Contains(t, err.Error(), "insufficient fee", "Error should mention insufficient fee") +} + +// TestVerifyActionFee_CompareWithClientSDK verifies that supernode uses +// same rounding as client SDK (from supernode/sdk/action/client.go:302-304) +func TestVerifyActionFee_CompareWithClientSDK(t *testing.T) { + testCases := []struct { + fileBytes int64 + expectedKB int64 + }{ + {0, 0}, + {1, 1}, + {1024, 1}, + {1025, 2}, + } + + for _, tc := range testCases { + t.Run(fmt.Sprintf("%d bytes", tc.fileBytes), func(t *testing.T) { + // Both client SDK and supernode now use the SAME centralized function + // Client uses RoundBytesToKB64 for int64 inputs + clientKB := int64(actiontypes.RoundBytesToKB64(tc.fileBytes)) + + // Supernode uses RoundBytesToKB for int inputs + supernodeKB := int64(actiontypes.RoundBytesToKB(int(tc.fileBytes))) + + assert.Equal(t, tc.expectedKB, clientKB, + "Client SDK rounding incorrect for %d bytes", tc.fileBytes) + assert.Equal(t, tc.expectedKB, supernodeKB, + "Supernode rounding incorrect for %d bytes", tc.fileBytes) + assert.Equal(t, clientKB, supernodeKB, + "Client and supernode rounding must match for %d bytes", tc.fileBytes) + + t.Logf("%d bytes → Client: %d KB, Supernode: %d KB %s", + tc.fileBytes, clientKB, supernodeKB, + map[bool]string{true: "✓ CONSISTENT", false: "✗ INCONSISTENT"}[clientKB == supernodeKB]) + }) + } +}