Skip to content

fixed fb kyc upload issue and other minor changes - #63

Merged
AnilChinchawale merged 2 commits into
XinFinOrg:mainfrom
rajshah1609:main
Jun 30, 2026
Merged

fixed fb kyc upload issue and other minor changes#63
AnilChinchawale merged 2 commits into
XinFinOrg:mainfrom
rajshah1609:main

Conversation

@rajshah1609

@rajshah1609 rajshah1609 commented Jun 30, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • New Features

    • App links, sitemap URLs, and tweet links now use the configured base URL, making deployments work correctly across environments.
  • Bug Fixes

    • Improved balance checks and account loading to handle alternate address formats more reliably.
    • Made voting, candidate, and KYC-related balance lookups more resilient when network calls fail.
    • Reduced transaction and session errors for wallet flows, including hardware wallet and WalletConnect handling.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@rajshah1609, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9931f76c-2e54-402e-8f21-72b08dca05e8

📥 Commits

Reviewing files that changed from the base of the PR and between fb61fd3 and 47e4a63.

📒 Files selected for processing (5)
  • apis/voters.js
  • app/app.js
  • app/components/candidates/Apply.vue
  • app/components/voters/View.vue
  • crawl.js
📝 Walkthrough

Walkthrough

Introduces a baseUrl config value consumed across server routes (CORS, sitemap, twitter, elect.js, config API) and the client app (WalletConnect, Trezor, analytics). Adds getBalanceSafe/getBalanceSafeCallback Vue prototype helpers with XDC/0x address fallback and a secondary RPC fallback, migrating all direct web3.eth.getBalance call sites. Hardens crawl.js RPC calls with try/catch guards.

Changes

baseUrl Config, Safe Balance Helpers, and RPC Hardening

Layer / File(s) Summary
Server-side baseUrl propagation
apis/config.js, index.js, middlewares/sitemap.js, helpers/twitter.js, elect.js
baseUrl from config is exposed in the config API response, added to the CORS allowlist, and replaces hardcoded domains in sitemap URL generation, tweet link construction, and the Electron window URL.
Client-side baseUrl for WalletConnect, Trezor, analytics
app/app.js
WalletConnect dappUrl, metadata.icons, and optionalChains are derived from configMaster.baseUrl; TrezorConnect.manifest and VueAnalytics linker domain are set dynamically from config.baseUrl at runtime.
getBalanceSafe / getBalanceSafeCallback implementation
app/app.js
Adds BigNumber import and two new Vue.prototype methods that fetch balances via the current provider then a fallback read-only RPC, querying both 0x and xdc address variants and preferring non-zero results.
Migrate call sites to safe balance helpers
apis/voters.js, app/components/Setting.vue, app/components/candidates/Apply.vue, app/components/candidates/View.vue, app/components/voters/View.vue, app/components/voters/Voting.vue
All direct web3.eth.getBalance calls in components are replaced with getBalanceSafe/getBalanceSafeCallback; the server-side voters.js error path adds an XDC address fallback to its balance check.
Ledger signature normalization and WalletConnect session hardening
app/app.js
formatLedgerSignature normalizes v/r/s uniformly; signMessage ledger branch uses toHexBuffer/toRpcSig; WalletConnect modal-close and resetWalletSession swallow close/disconnect errors; getAccount and detectNetwork are refactored in place; XDC HD path returns XDC symbol.
crawl.js RPC error guards
crawl.js
updateCandidateInfo, updateVoterCap, getCurrentCandidates, and updateSignerPenAndStatus wrap RPC calls in try/catch, log errors, and return early or use defaults on failure; candidate status update uses a computed newStatus with owner fallback.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • XinFinOrg/MasterNode-App#48: Both modify index.js CORS handling, replacing the static origin check with an origin callback supporting conditional allowed origins.
  • XinFinOrg/MasterNode-App#59: Both modify WalletConnect session/modal handling in app/app.js and Setting.vue balance loading during login using alternative Web3 paths.
  • XinFinOrg/MasterNode-App#60: Both modify Apply.vue's uploadKYC flow sender balance/insufficiency checking.

Suggested reviewers

  • RushabhParmar12
  • AnilChinchawale

🐇 Hopping through configs with glee,
No more hardcoded URLs to see!
Balance checks now safely land,
With fallback RPC close at hand,
*And crawl.js caught each RPC plea ~*

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references the KYC upload fix reflected in Apply.vue, and while broad, it still matches the main theme of the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/components/voters/View.vue (1)

674-678: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Do not throw from the async balance callback.

This throw is outside the surrounding try/catch and can become an unhandled async error. Log the error and keep the safe "0" fallback instead.

Proposed fix
                     self.getBalanceSafeCallback(voter, 'voters/View.vue - created', function (a, b) {
-                        self.balance = new BigNumber(b).div(10 ** 18).toNumber()
                         if (a) {
-                            throw Error(a)
+                            console.log('got an error', a)
+                            return
                         }
+                        self.balance = new BigNumber(b).div(10 ** 18).toNumber()
                     })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/components/voters/View.vue` around lines 674 - 678, In View.vue’s created
balance callback inside getBalanceSafeCallback, do not throw when a is present
because it escapes the surrounding try/catch and can surface as an unhandled
async error. Update the callback to log the error through the existing error
handling path, and preserve the safe "0" fallback behavior in self.balance
assignment so the voter view still renders safely.
🧹 Nitpick comments (1)
app/app.js (1)

725-733: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Wrap both case declarations in blocks. Biome’s noSwitchDeclarations rule applies here and in signMessage; adding {} around each case keeps the lexical scope explicit.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/app.js` around lines 725 - 733, Wrap the declarations in the trezor
branch of the switch in a block, since Biome’s noSwitchDeclarations rule is
triggered by lexical bindings directly under a case label. Update the trezor
case in the same way as signMessage by enclosing the case body in braces so
trezorPayload, trezorOffset, and account stay within an explicit scope.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apis/voters.js`:
- Around line 267-275: The logging in sendSignedTransaction is exposing
sensitive wallet data, including signedAddress, action/amount, and exact
balances from web3.eth.getBalance. Update this flow to remove or redact these
values from normal server logs, and only emit them behind an explicit debug flag
or debug-level logger. Keep the diagnostic context in sendSignedTransaction, but
ensure the console.log statements in the vote path no longer print raw addresses
or precise balance amounts.
- Around line 271-277: The balance selection in voters.js is incorrectly using
the 0x result directly in the fallback expression, so a `"0"` balance blocks the
XDC balance and any getBalance failure prevents fallback entirely. Update the
logic around the signedAddress/xdcAddress balance lookup so the code explicitly
treats a zero 0x balance as empty and falls back to the XDC value, and wrap the
web3.eth.getBalance calls in a way that still uses balanceXdc if the 0x lookup
throws. Keep the change near the existing getBalance logging and the final
balance assignment.

In `@app/app.js`:
- Around line 839-880: The current getBalanceSafe flow in
Vue.prototype.getBalanceSafe swallows every getBalance error and returns '0'
even when all RPC reads fail. Update the method to track whether any balance
lookup succeeded across both this.web3 and getChainReadWeb3, and if none do,
throw an error instead of returning zero. Keep the existing zero-vs-fallback
selection logic for successful reads, and ensure getBalanceSafeCallback can
surface the thrown failure path.
- Around line 515-522: The wallet reset flow in resetWalletConnectProvider is
not handling the Promise returned by instance.disconnect(), so a rejection can
escape even though it is wrapped in try/catch. Update the disconnect handling in
resetWalletConnectProvider to await the returned Promise or attach a catch
handler, and make sure any failure is swallowed or logged consistently with the
existing closeWalletConnectModal(instance).catch(...) behavior.

In `@app/components/candidates/Apply.vue`:
- Around line 670-674: The fee balance check in Apply.vue’s KYC upload flow
happens too late, after the KYC file has already been posted. Add a conservative
preflight balance check in the same uploadKYC path before the file upload step,
using getBalanceSafe, gas, and gasPrice to estimate the required fee. If the
account balance is insufficient, stop early and surface the existing error path
so users do not send KYC data that cannot be anchored on-chain.

In `@crawl.js`:
- Around line 201-205: The status/owner fallback logic in getCurrentCandidates
is reading from candateInDB after the DB record has already been removed, so it
loses the pre-removal snapshot. Update the status resolution to use the captured
previous candidate state (prevCandidates/prevStatus) instead of
candateInDB.status, and pass the captured owner through as the fallback source
when the owner RPC fails. Make the same snapshot-based fallback change in the
related owner handling code so reactivated candidates don’t stay RESIGNED and
existing owner values aren’t cleared.
- Around line 380-385: The candidate node response validation in crawl.js is too
shallow: `candidateAddressData.data.result` can be present while `Masternodes`,
`Penalty`, or `Standbynodes` are missing or not arrays, which later breaks the
`.includes` checks. Update the validation around the
`candidateAddressData.data.result` destructuring in the
`XDPoS_getMasternodesByNumber` flow to verify each of `masterNodes`,
`slashNodes`, and `standByNodes` is a real array before continuing, and
log/return early when any are invalid.
- Around line 372-378: The RPC call in the crawler currently uses axios.post
without any request timeout, so an unresponsive endpoint can hang the flow
before rpcErr is ever caught. Update the axios.post call in the
candidateAddressData fetch block to include a reasonable timeout via the request
config, and keep the existing catch/logging path in place so the crawler fails
fast and continues handling RPC errors cleanly.

---

Outside diff comments:
In `@app/components/voters/View.vue`:
- Around line 674-678: In View.vue’s created balance callback inside
getBalanceSafeCallback, do not throw when a is present because it escapes the
surrounding try/catch and can surface as an unhandled async error. Update the
callback to log the error through the existing error handling path, and preserve
the safe "0" fallback behavior in self.balance assignment so the voter view
still renders safely.

---

Nitpick comments:
In `@app/app.js`:
- Around line 725-733: Wrap the declarations in the trezor branch of the switch
in a block, since Biome’s noSwitchDeclarations rule is triggered by lexical
bindings directly under a case label. Update the trezor case in the same way as
signMessage by enclosing the case body in braces so trezorPayload, trezorOffset,
and account stay within an explicit scope.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 69ef5d08-5799-4078-90bd-83b4922a2f66

📥 Commits

Reviewing files that changed from the base of the PR and between 52633be and fb61fd3.

📒 Files selected for processing (14)
  • apis/config.js
  • apis/voters.js
  • app/app.js
  • app/components/Setting.vue
  • app/components/candidates/Apply.vue
  • app/components/candidates/View.vue
  • app/components/voters/View.vue
  • app/components/voters/Voting.vue
  • crawl.js
  • elect.js
  • helpers/personalSign.js
  • helpers/twitter.js
  • index.js
  • middlewares/sitemap.js

Comment thread apis/voters.js Outdated
Comment thread apis/voters.js Outdated
Comment thread app/app.js
Comment thread app/app.js
Comment thread app/components/candidates/Apply.vue
Comment thread crawl.js Outdated
Comment thread crawl.js
Comment thread crawl.js
@AnilChinchawale
AnilChinchawale merged commit 85fd1e3 into XinFinOrg:main Jun 30, 2026
1 check passed
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.

3 participants