Escrow management#518
Merged
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new escrow management dashboard at ChangesEscrow Dashboard and Payment Flow Consolidation
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over EscrowPage,OceanProvider: Page Load & Data Fetch
EscrowPage->>useEscrowData: mount → load()
useEscrowData->>OceanProvider: getSupportedTokens()
OceanProvider-->>useEscrowData: token list
useEscrowData->>OceanProvider: getUserFundsDetailed(token, user)
OceanProvider-->>useEscrowData: available, locked balances
useEscrowData->>nodeService: getEscrowEvents(Auth, payer)
nodeService-->>useEscrowData: EscrowEvent[] (spender addresses)
useEscrowData->>OceanProvider: getLocks(token, payer, spender)
OceanProvider-->>useEscrowData: EscrowLock[]
useEscrowData-->>EscrowPage: tokens, spenders, loading, reload
end
rect rgba(144, 238, 144, 0.5)
Note over EscrowTokenPanel,OceanProvider: Create & Edit Authorization
EscrowTokenPanel->>CreateAuthorizationModal: open(tokenAddress, existingConsumers)
CreateAuthorizationModal->>nodeService: getNodes(nodeId)
nodeService-->>CreateAuthorizationModal: node + consumerAddress
CreateAuthorizationModal->>useAuthorizeTokens: handleAuthorize(tokenAddress, peerId, spender, lockParams)
useAuthorizeTokens->>OceanProvider: authorize on-chain
OceanProvider-->>useAuthorizeTokens: success
useAuthorizeTokens-->>CreateAuthorizationModal: toast(authorizationSuccessMessage)
CreateAuthorizationModal-->>EscrowTokenPanel: onSuccess() → reload()
end
rect rgba(255, 182, 193, 0.5)
Note over EscrowTokenPanel,OceanProvider: Deposit & Withdraw
EscrowTokenPanel->>useDepositTokens: handleDeposit(amount)
useDepositTokens->>OceanProvider: approve + deposit
OceanProvider-->>useDepositTokens: success
useDepositTokens-->>EscrowTokenPanel: onSuccess() → reload()
EscrowTokenPanel->>useWithdrawTokens: handleWithdraw(amount)
useWithdrawTokens->>OceanProvider: withdraw
OceanProvider-->>useWithdrawTokens: success
useWithdrawTokens-->>EscrowTokenPanel: onSuccess() → reload()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
andreip136
requested review from
alexcos20,
bogdanfazakas,
dnsi0,
giurgiur99,
mihaisc and
ndrpp
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/run-job/payment-summary.tsx (1)
31-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix allowance insufficiency calculation when authorization data is missing.
At Line 31,
Number(authorizations?.maxLockedAmount) ?? 0can produceNaN, and?? 0won’t apply toNaN. That makes the check false and hides the “Insufficient allowance” state at Lines 66-70 when authorizations are absent/invalid.Suggested fix
- const insufficientAutorized = (Number(authorizations?.maxLockedAmount) ?? 0) < totalCost; + const maxLockedAmount = Number(authorizations?.maxLockedAmount ?? 0); + const insufficientAutorized = !Number.isFinite(maxLockedAmount) || maxLockedAmount < totalCost;Also applies to: 64-70
🤖 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 `@src/components/run-job/payment-summary.tsx` at line 31, The `insufficientAutorized` variable calculation on line 31 has a logic error where `Number(authorizations?.maxLockedAmount) ?? 0` can produce NaN when authorization data is invalid, and the nullish coalescing operator does not catch NaN values, causing the comparison to fail silently. Fix this by restructuring the expression to ensure a valid number is always produced, such as using `Number(authorizations?.maxLockedAmount || 0)` which handles undefined/invalid values before conversion, allowing the insufficient allowance message at lines 66-70 to display correctly when authorization data is missing or invalid.
🤖 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.
Outside diff comments:
In `@src/components/run-job/payment-summary.tsx`:
- Line 31: The `insufficientAutorized` variable calculation on line 31 has a
logic error where `Number(authorizations?.maxLockedAmount) ?? 0` can produce NaN
when authorization data is invalid, and the nullish coalescing operator does not
catch NaN values, causing the comparison to fail silently. Fix this by
restructuring the expression to ensure a valid number is always produced, such
as using `Number(authorizations?.maxLockedAmount || 0)` which handles
undefined/invalid values before conversion, allowing the insufficient allowance
message at lines 66-70 to display correctly when authorization data is missing
or invalid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9ceed499-62b5-450b-8c78-d85da05ba33b
📒 Files selected for processing (5)
src/components/escrow/escrow-token-panel.tsxsrc/components/run-job/payment-summary.module.csssrc/components/run-job/payment-summary.tsxsrc/lib/use-escrow-data.tssrc/services/nodeService.ts
✅ Files skipped from review due to trivial changes (1)
- src/components/run-job/payment-summary.module.css
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/use-escrow-data.ts
- src/components/escrow/escrow-token-panel.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/use-ocean-account.tsx`:
- Around line 176-179: The `ocean` instance in use-ocean-account.tsx is
initialized with a read-only JsonRpcProvider, but the methods
`authorizeTokensEoa`, `depositTokensEoa`, and `withdrawTokensEoa` all internally
call `this.provider.getSigner()` which fails on read-only providers. Fix this by
either: (1) modifying the ocean provider initialization to use the wallet's
BrowserProvider (the `provider` variable defined earlier in the hook) for write
operations while keeping the read-only provider for read-only calls, or (2)
refactor the three escrow write methods to use `sendTransaction` from the wallet
context with encoded contract calls instead of relying on the ocean instance's
signer-dependent methods.
🪄 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 Plus
Run ID: a0f11014-02b2-4e50-a7c9-60d88dd57c5b
📒 Files selected for processing (1)
src/lib/use-ocean-account.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/node-details/node-info.tsx (1)
82-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard condition checks
node?.idbut operation usesnodeId.The guard checks
!node?.id, but the subsequent operations usenodeIdwhich is derived fromnode.id ?? node.nodeId. If a node has onlynodeId(notid), this function returns early even thoughnodeIdholds a valid value.The same pattern appears in
handlePushConfig(line 107) andhandleDownloadLogs(line 150).🐛 Proposed fix
async function handleFetchConfig() { if (!account.isConnected) { login(); return; } - if (!ocean || !node?.id) { + if (!ocean || !nodeId) { return; }Apply the same fix to
handlePushConfigandhandleDownloadLogs.🤖 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 `@src/components/node-details/node-info.tsx` around lines 82 - 84, The guard condition checks `!node?.id` but the function uses `nodeId` which is derived from `node.id ?? node.nodeId`. This causes the function to return early even when `nodeId` contains a valid value (when node only has nodeId but not id). Update the guard condition to check `!nodeId` instead of `!node?.id` to properly validate the value actually being used. Apply this same fix pattern to both the `handlePushConfig` method and the `handleDownloadLogs` method where the identical issue occurs.src/lib/use-authorize-tokens.ts (1)
50-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent stuck loading when the Ocean client is unavailable.
At Line 54, the EOA path returns early after
setIsAuthorizing(true)(Line 53), leavingisAuthorizingstucktrue. This can lock authorization UI indefinitely.Proposed fix
async ({ tokenAddress, peerId, spender, maxLockedAmount, maxLockSeconds, maxLockCount }: AuthorizeTokensParams) => { if (user?.type === 'eoa') { try { - setIsAuthorizing(true); - if (!ocean) return; + if (!ocean) { + setError('Ocean client not initialized'); + toast.error('Authorization failed'); + return; + } + setIsAuthorizing(true); const tx = await ocean.authorizeTokensEoa({ tokenAddress, spender,🤖 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 `@src/lib/use-authorize-tokens.ts` around lines 50 - 55, The early return statement in the EOA authorization path (when checking if !ocean) occurs after setIsAuthorizing(true) has been called, which leaves the isAuthorizing state stuck in true state indefinitely. Before returning early when the ocean client is unavailable, reset the isAuthorizing state to false by calling setIsAuthorizing(false) to ensure the loading state does not persist and lock the authorization UI.
🤖 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 `@src/components/escrow/create-authorization-modal.tsx`:
- Around line 58-75: The handleAuthorize function call in the onSubmit callback
is async but not being awaited, which allows Formik to clear the isSubmitting
state before the authorization transaction completes, potentially reopening the
submit path during latency. Add the await keyword before the handleAuthorize
call to ensure Formik keeps isSubmitting true until the authorization lifecycle
completes, preventing duplicate submissions during transaction processing.
- Around line 84-85: In the axios GET request where the node lookup is
performed, replace the manually constructed URL string with proper parameter
encoding. Instead of interpolating nodeId directly into the URL string in the
axios.get call for the nodes endpoint, use Axios's params configuration option
to pass all query parameters (page, size, and nodeId) as an object in the second
argument. This ensures proper URL encoding and prevents malformed requests.
---
Outside diff comments:
In `@src/components/node-details/node-info.tsx`:
- Around line 82-84: The guard condition checks `!node?.id` but the function
uses `nodeId` which is derived from `node.id ?? node.nodeId`. This causes the
function to return early even when `nodeId` contains a valid value (when node
only has nodeId but not id). Update the guard condition to check `!nodeId`
instead of `!node?.id` to properly validate the value actually being used. Apply
this same fix pattern to both the `handlePushConfig` method and the
`handleDownloadLogs` method where the identical issue occurs.
In `@src/lib/use-authorize-tokens.ts`:
- Around line 50-55: The early return statement in the EOA authorization path
(when checking if !ocean) occurs after setIsAuthorizing(true) has been called,
which leaves the isAuthorizing state stuck in true state indefinitely. Before
returning early when the ocean client is unavailable, reset the isAuthorizing
state to false by calling setIsAuthorizing(false) to ensure the loading state
does not persist and lock the authorization UI.
🪄 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 Plus
Run ID: f26535d4-8344-4996-b37a-e828ba07da20
📒 Files selected for processing (5)
src/components/escrow/create-authorization-modal.tsxsrc/components/node-details/node-info.module.csssrc/components/node-details/node-info.tsxsrc/components/run-job/payment-authorize.tsxsrc/lib/use-authorize-tokens.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/run-job/payment-authorize.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This reverts commit 03ae23f.
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Draft
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/lib/use-escrow-data.ts (1)
70-95: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winKeep spender loading resilient to per-payee RPC failures.
Line 73 uses
Promise.all(...)for all payees of a token. One failedgetLocksrejects the entire token batch, dropping otherwise valid spender rows.💡 Suggested patch
const settled = await Promise.allSettled( tokenList.map(async (token) => { const allAuthorizations = await ocean.getAllAuthorizations(token.address, account.address!); - return Promise.all( + const perPayee = await Promise.allSettled( allAuthorizations.map(async (authorizations) => { const spender = authorizations.payee; const locks = await ocean.getLocks(token.address, account.address!, spender); return { tokenSymbol: token.symbol, tokenAddress: token.address, spender, authorizations, locks, }; }) ); + return perPayee + .filter((r): r is PromiseFulfilledResult<EscrowSpenderInfo> => r.status === 'fulfilled') + .map((r) => r.value); }) );🤖 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 `@src/lib/use-escrow-data.ts` around lines 70 - 95, The inner Promise.all call that maps over allAuthorizations is causing the entire token batch to fail if any single getLocks call fails for a payee. Replace the Promise.all with Promise.allSettled for the inner mapping over allAuthorizations, then filter the results to keep only the fulfilled items and extract their values. This ensures that if one payee's getLocks fails, the other valid spender rows for that token are still included in the results instead of the entire token batch being dropped.
🤖 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 `@src/components/escrow/revoke-authorization-modal.tsx`:
- Around line 58-63: The Button component in the revoke authorization modal has
a disabled prop that only checks hasActiveLocks, but it doesn't prevent
interaction while isAuthorizing is true. This allows users to trigger multiple
revoke transactions. Update the disabled prop to combine both conditions so the
button is disabled when either hasActiveLocks is true OR isAuthorizing is true,
ensuring the button remains disabled during the entire authorization process.
In `@src/lib/use-escrow-data.ts`:
- Around line 15-18: The current filtering logic uses the isRevokedAuthorization
function to remove rows at lines 95-96, but this suppresses revoked
authorizations that still have active locks, causing the Escrow locks list to be
incomplete. Modify the filter condition to not only check if an authorization is
revoked but also verify whether it still has active locks remaining; only remove
the row if it is revoked AND has no active locks, ensuring that any
authorization with ongoing locked amounts is retained in the list regardless of
its revocation status.
---
Duplicate comments:
In `@src/lib/use-escrow-data.ts`:
- Around line 70-95: The inner Promise.all call that maps over allAuthorizations
is causing the entire token batch to fail if any single getLocks call fails for
a payee. Replace the Promise.all with Promise.allSettled for the inner mapping
over allAuthorizations, then filter the results to keep only the fulfilled items
and extract their values. This ensures that if one payee's getLocks fails, the
other valid spender rows for that token are still included in the results
instead of the entire token batch being dropped.
🪄 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 Plus
Run ID: 0d8ede9c-12a2-41c0-b956-e7a2b4875745
📒 Files selected for processing (5)
src/components/escrow/authorization-form.module.csssrc/components/escrow/escrow-token-panel.tsxsrc/components/escrow/revoke-authorization-modal.tsxsrc/lib/ocean-provider.tssrc/lib/use-escrow-data.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/escrow/escrow-token-panel.tsx
- src/lib/ocean-provider.ts
giurgiur99
approved these changes
Jun 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #532 , #533 .
Changes proposed in this PR:
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes