Skip to content

Conversation

@brunocalza
Copy link
Member

@brunocalza brunocalza commented Sep 9, 2025

Context

Closes APP-318

Adds the following 3 endpoints:

  1. GET /user/rewards/total
  2. GET /user/rewards/proofs
  3. POST /user/rewards/claim/

These will be used to implement the CLAIM flow button in the navbar:

image

@vercel
Copy link
Contributor

vercel bot commented Sep 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
comps Ready Ready Preview Comment Oct 6, 2025 0:54am
comps-staging2 Ready Ready Preview Comment Oct 6, 2025 0:54am
portal Ready Ready Preview Comment Oct 6, 2025 0:54am

"0x1234567890123456789012345678901234567890" as `0x${string}`;
const testStartTimestamp = Math.floor(Date.now() / 1000); // Current timestamp

beforeEach(async () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Before each test, we add some entries in the rewards table. We also call the mocked allocate call that will add entries to rewards_tree and rewards_roots tables. With that data in the database, we can test the 3 endpoints.

Copy link
Contributor

@joewagner joewagner left a comment

Choose a reason for hiding this comment

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

Nice!
I put a few nits inline, but looks good

@github-actions
Copy link
Contributor

github-actions bot commented Sep 22, 2025

📊 Test Coverage Report

Package Lines Statements Functions Branches
apps/api 3.53% (-0.02%) 3.53% (-0.02%) 43.80% (+0.47%) 50.79% (+0.26%)
apps/comps 0.00% 0.00% 40.89% 40.89%
packages/conversions 100.00% 100.00% 100.00% 100.00%
packages/db 0.00% 0.00% 16.27% 16.27%
packages/rewards 100.00% 100.00% 100.00% 100.00%
packages/services 35.90% (-0.12%) 35.90% (-0.12%) 63.81% (-0.36%) 77.94%
packages/staking-contracts 100.00% 100.00% 100.00% 100.00%

.from(rewards)
.where(
and(
eq(rewards.address, address.toLocaleLowerCase()),
Copy link
Contributor

Choose a reason for hiding this comment

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

The method uses address.toLocaleLowerCase() instead of address.toLowerCase() for Ethereum address normalization, which is incorrect and inconsistent with the codebase pattern.

View Details
📝 Patch Details
diff --git a/packages/db/src/repositories/rewards.ts b/packages/db/src/repositories/rewards.ts
index 5c34101..b0a7ae5 100644
--- a/packages/db/src/repositories/rewards.ts
+++ b/packages/db/src/repositories/rewards.ts
@@ -199,7 +199,7 @@ export class RewardsRepository {
         .from(rewards)
         .where(
           and(
-            eq(rewards.address, address.toLocaleLowerCase()),
+            eq(rewards.address, address.toLowerCase()),
             eq(rewards.claimed, false),
           ),
         );
@@ -236,7 +236,7 @@ export class RewardsRepository {
         )
         .where(
           and(
-            eq(rewards.address, address.toLocaleLowerCase()),
+            eq(rewards.address, address.toLowerCase()),
             eq(rewards.claimed, false),
           ),
         );

Analysis

Inconsistent address normalization method in RewardsRepository

What fails: RewardsRepository uses toLocaleLowerCase() instead of toLowerCase() for Ethereum address normalization, creating inconsistency with other repositories

How to reproduce:

// In packages/db/src/repositories/rewards.ts lines 202 and 239:
eq(rewards.address, address.toLocaleLowerCase()) // Inconsistent method
// vs other repositories like agent.ts, user.ts:
eq(users.walletAddress, normalizedWalletAddress.toLowerCase()) // Standard method

Result: Code inconsistency across repositories using different case conversion methods

Expected: All repositories should use toLowerCase() for Ethereum address normalization per codebase patterns in AgentRepository and UserRepository

Note: While both methods produce identical results for hexadecimal Ethereum addresses, toLowerCase() is the established pattern and avoids potential locale-specific behavior per MDN documentation

Signed-off-by: Bruno Calza <[email protected]>
@brunocalza brunocalza merged commit 1fe0969 into main Oct 6, 2025
12 checks passed
@brunocalza brunocalza deleted the bcalza/rewards-api branch October 6, 2025 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants