-
Notifications
You must be signed in to change notification settings - Fork 9
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
added some necessary improvements (#195) #196
Conversation
WalkthroughThis pull request introduces several updates to the backend, including a version increment to 3.1.5, a new error message constant for empty request bodies, and a getter method for the Changes
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
backend/CHANGELOG.md
(1 hunks)backend/package.json
(1 hunks)backend/src/constants/ErrorMessage.ts
(1 hunks)backend/src/models/sponsorship-policy.ts
(1 hunks)backend/src/routes/deposit-route.ts
(2 hunks)backend/src/server.ts
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
backend/CHANGELOG.md
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
4-4: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
backend/package.json (1)
3-3
: Version bump looks appropriate.The version increment from 3.1.3 to 3.1.4 aligns with the bugfix nature of the PR.
backend/src/constants/ErrorMessage.ts (1)
2-2
: Good addition for better error handling.This new error message will improve error handling for empty request bodies in the deposit endpoints.
backend/src/server.ts (1)
44-44
: Makes all routes case-insensitive as documented in changelog.Setting
caseSensitive: false
in the Fastify server configuration ensures all API endpoints are case-insensitive, which improves the API's usability.backend/src/routes/deposit-route.ts (2)
50-52
: Good error handling improvement.Adding validation for empty request bodies is a great defensive programming practice. This prevents potential issues that could occur when trying to access properties of undefined objects later in the code flow.
123-125
: Good consistency in error handling across endpoints.This validation matches the one added to the first endpoint, ensuring consistent error handling throughout the API. The error response uses the appropriate return code and a specific error message, which aligns with good API design practices.
backend/src/models/sponsorship-policy.ts (1)
152-155
: Good improvement for consistent type handling.Adding a getter to convert the
globalMaximumUsd
decimal value to a JavaScript Number ensures consistent type handling when performing calculations. The null check is also properly implemented.While converting to Number could theoretically lose precision for very large decimal values, the DECIMAL(10,4) format used here should be adequately represented by JavaScript's Number type for currency values.
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.
LGTM
* added all token decimals * added gnosis for epv7 as supported networks
Deploying arka with
|
Latest commit: |
c474929
|
Status: | ✅ Deploy successful! |
Preview URL: | https://0fa32ecb.arka-3qg.pages.dev |
Branch Preview URL: | https://develop.arka-3qg.pages.dev |
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/CHANGELOG.md (1)
2-5
: 🧹 Nitpick (assertive)Consider adding blank lines around headings for Markdown compliance.
Markdown linting rules (MD022) often require one blank line before and after headings. You can fix it by adding blank lines around
## [3.1.5] - 2025-03-24
and### Fixes
, for example:2 ## [3.1.5] - 2025-03-24 3 ### Fixes 4 - Added all tokens used in multiTokenPaymaster to have decimals in constants rather than fetching from rpc 5 +## [3.1.5] - 2025-03-24 +### Fixes +- Added all tokens used in multiTokenPaymaster to have decimals in constants rather than fetching from rpc🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above(MD022, blanks-around-headings)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below(MD022, blanks-around-headings)
4-4: Trailing spaces
Expected: 0 or 2; Actual: 1(MD009, no-trailing-spaces)
4-4: Lists should be surrounded by blank lines
null(MD032, blanks-around-lists)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
backend/CHANGELOG.md
(1 hunks)backend/config.json.default
(1 hunks)backend/package.json
(1 hunks)backend/src/constants/MultitokenPaymaster.ts
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
backend/CHANGELOG.md
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
4-4: Trailing spaces
Expected: 0 or 2; Actual: 1
(MD009, no-trailing-spaces)
4-4: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
8-8: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (10)
backend/package.json (1)
3-3
: Version bump looks good.Incrementing the version from 3.1.3 to 3.1.5 aligns with your changelog entry. No issues spotted here.
backend/config.json.default (1)
122-132
: Confirm the empty Paymaster address for the new chainId 100 entry.You’ve added a second entry for the same
chainId
: 100, with av2
bundler URL. Notably,"etherspotPaymasterAddress"
is left empty. If this is intentional, please confirm that the application can handle an empty address without error.Would you like a script to check how this new chain config is referenced across the codebase for potential issues?
backend/src/constants/MultitokenPaymaster.ts (8)
11-19
: Verify token addresses & decimals for chainId=1.Please ensure all contract addresses (e.g., Tether, USDC, LINK, etc.) match official sources and decimals are correct. Incorrect addresses or decimal values could lead to payment failures.
21-40
: Check newly added tokens for chainId=10.This chunk includes many tokens (KWENTA, LYRA, etc.). Verify that each contract address and decimal setting is valid and up to date.
42-104
: Large addition of tokens for chainId=56 (BSC).You’ve added numerous entries (e.g., USDT, USDC, MATIC, DOGE). Confirm the correctness of each address, as any mismatch could break paymaster logic or cause incorrect fee deductions.
105-148
: New tokens for chainId=137 (Polygon).Entries for BUSD, USDC, AAVE, etc., have been introduced or restructured. Ensure these addresses are accurate and decimals reflect actual token configurations on Polygon.
150-154
: chainId=204 tokens updated.Entries like
BTCB
,ETH
,USDT
, andFDUSD
were added. Please verify each address and decimal.
155-163
: Validate newly introduced addresses for chainId=8453.Important tokens like
USDbC
,cbETH
, andrETH
are added. Ensure these addresses are correct, as they differ from standard Ethereum networks.
164-193
: Review chainId=42161 (Arbitrum) tokens carefully.This set includes standard stablecoins (USDT, USDC) alongside specialized tokens (VELA, GMX, STG, etc.). Mistakes can cause immediate failures in paymaster functionalities.
195-202
:❓ Verification inconclusive
Confirm addresses for chainId=43114 (Avalanche).
Tokens such as GMX, USDT, and others are newly configured. If you need help verifying them, consider a script that cross-checks addresses or searching block explorers for official references.
🏁 Script executed:
#!/bin/bash # Attempt to locate occurrences of newly introduced Avalanche addresses in the codebase # and see if they match known references from explorers like snowtrace.io addresses=( "0x62edc0692BD897D2295872a9FFCac5425011c661" "0x9702230A8Ea53601f5cD2dc00fDBc13d4dF4A8c7" "0xD24C2Ad096400B6FBcd2ad8B24E7acBc21A1da64" "0xB97EF9Ef8734C71904D8002F8b6Bc66Dd9c48a6E" "0xC891EB4cbdEFf6e073e859e987815Ed1505c2ACD" "0xB3fe5374F67D7a22886A0eE082b2E2f9d2651651" ) for addr in "${addresses[@]}"; do echo "Searching for $addr in the repo:" rg "$addr" doneLength of output: 2451
Avalanche Token Address Confirmation Needed
The addresses inbackend/src/constants/MultitokenPaymaster.ts
(covering GMX, USDt, FRAX, USDC, EURC, LINK, and UNI.e) are currently defined only here. Although our search confirms that each address is uniquely referenced in this file, please verify these addresses against official Avalanche sources (e.g., Snowtrace) to ensure they are correct and up to date.
- Confirm that all token addresses (especially for USDt and UNI.e, which were not part of the initial search batch) match their official deployments on Avalanche (chainId=43114).
- Cross-check with block explorer references or official token documentation to avoid any discrepancies with these newly configured tokens.
## [3.1.4] - 2025-03-19 | ||
### Fixes | ||
- Changed all endpoints to be case-insensitive | ||
- Fetch `globalMaximumUsd` value to convert into Number for absolute value | ||
- Improve error handling in deposit apis |
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.
🧹 Nitpick (assertive)
Apply similar markdown spacing improvements here.
Just like the above segment, you can address the same markdown lint warnings (MD022, MD032) by surrounding headings and lists with blank lines. This will keep your changelog consistent and pass lint checks.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
6-6: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
7-7: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
8-8: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit
New Features
Bug Fixes
Chores