-
Notifications
You must be signed in to change notification settings - Fork 13
update dependencies #232
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
update dependencies #232
Conversation
WalkthroughDependency version bump of Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Deploying wallet-signet with
|
| Latest commit: |
4aea271
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2bdfa693.wallet-23u.pages.dev |
| Branch Preview URL: | https://revert-kukks.wallet-23u.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
4aea271
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1df4a3bf.arkade-wallet.pages.dev |
| Branch Preview URL: | https://revert-kukks.arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
4aea271
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0e6c3901.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://revert-kukks.wallet-bitcoin.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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(1 hunks)src/test/screens/wallet/send.test.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-30T18:33:29.839Z
Learnt from: bordalix
Repo: arkade-os/wallet PR: 114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.
Applied to files:
src/test/screens/wallet/send.test.tsx
🧬 Code graph analysis (1)
src/test/screens/wallet/send.test.tsx (2)
src/providers/flow.tsx (1)
FlowContext(89-104)src/test/screens/mocks.ts (1)
mockFlowContextValue(79-92)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (1)
package.json (1)
8-8: Verify the SDK upgrade manually—public release notes unavailable.No security advisories were found for @arkade-os/sdk. However, public release notes for v0.3.7 could not be located, and the package repository information is not available through standard sources. Manually verify the upgrade by checking the package source directly (e.g., npm package page, GitHub releases, or package documentation) to confirm any behavioral changes and compatibility with your codebase, particularly given the test modifications in this PR.
| <LightningContext.Provider value={mockLightningContextValue as any}> | ||
| <OptionsContext.Provider value={mockOptionsContextValue as any}> | ||
| <FlowContext.Provider value={mockFlowContextValue}> | ||
| <FlowContext.Provider value={mockFlowContextValue as any}> |
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.
🛠️ Refactor suggestion | 🟠 Major
Fix the type error instead of suppressing it with as any.
The type cast hides a legitimate type mismatch. Looking at the relevant code snippets, mockFlowContextValue is missing the deepLinkInfo property that FlowContextProps requires.
In src/test/screens/mocks.ts, add the missing property to the mock:
export const mockFlowContextValue = {
txInfo: mockTxInfo,
swapInfo: undefined,
initInfo: emptyInitInfo,
noteInfo: emptyNoteInfo,
deepLinkInfo: undefined, // Add this line
recvInfo: emptyRecvInfo,
sendInfo: emptySendInfo,
setInitInfo: () => {},
setNoteInfo: () => {},
setDeepLinkInfo: () => {}, // Add this if missing
setRecvInfo: () => {},
setSendInfo: () => {},
setSwapInfo: () => {},
setTxInfo: () => {},
}Then remove the as any casts from lines 35 and 60:
- <FlowContext.Provider value={mockFlowContextValue as any}>
+ <FlowContext.Provider value={mockFlowContextValue}>Also applies to: 60-60
🤖 Prompt for AI Agents
In src/test/screens/wallet/send.test.tsx around lines 35 and 60, the test is
using "mockFlowContextValue as any" to silence a type mismatch; instead add the
missing deepLinkInfo property (and its setter) to the mockFlowContextValue in
src/test/screens/mocks.ts so the object conforms to FlowContextProps, then
remove the "as any" casts on lines 35 and 60; ensure deepLinkInfo is present
(can be undefined) and the corresponding setDeepLinkInfo is a noop function so
the mock satisfies the required type.
| // button continue should be present | ||
| expect(screen.getByText('Continue')).toBeInTheDocument() | ||
| expect(screen.getByText('Amount below 1 satoshi')).toBeInTheDocument() |
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.
🧩 Analysis chain
Update the outdated comment and verify the behavioral change.
The comment on line 82 states "button continue should be present", but the assertion now checks for validation text "Amount below 1 satoshi" instead. This suggests the UI behavior has changed, possibly due to the SDK upgrade introducing stricter amount validation.
Update the comment to reflect the current behavior:
- // button continue should be present
+ // validation message should appear for amounts below 1 satoshi
expect(screen.getByText('Amount below 1 satoshi')).toBeInTheDocument()Additionally, verify this behavioral change is intentional by checking the SendForm component to confirm the validation logic was updated:
🏁 Script executed:
#!/bin/bash
# Find the validation logic for minimum satoshi amount
ast-grep --pattern 'below 1 satoshi'Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for satoshi validation logic in the codebase
rg -i "satoshi" --type=ts --type=tsx -B 3 -A 3Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for SendForm component to verify validation implementation
fd "SendForm" --type=ts --type=tsx --exec grep -l "satoshi\|Amount below" {} \;Length of output: 169
🏁 Script executed:
#!/bin/bash
# Search more broadly for the validation message
rg "Amount below" --type=ts --type=tsx -B 2 -A 2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for satoshi validation without type restrictions
rg "satoshi" -B 3 -A 3Length of output: 37205
🏁 Script executed:
#!/bin/bash
# Search for the validation message text
rg "Amount below" -B 2 -A 2Length of output: 2437
🏁 Script executed:
#!/bin/bash
# Find SendForm component
fd "SendForm" -e ts -e tsxLength of output: 0
Update the outdated comment to reflect the current validation behavior.
The comment on line 82 is outdated. The test now verifies that a validation message appears when the amount is below 1 satoshi, not that a "Continue" button is present. This behavioral change is intentional—the SendForm component in src/screens/Wallet/Send/Form.tsx includes validation logic that sets "Amount below 1 satoshi" as the button label when satoshis < 1.
Update the comment:
- // button continue should be present
+ // validation message should appear for amounts below 1 satoshi
expect(screen.getByText('Amount below 1 satoshi')).toBeInTheDocument()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // button continue should be present | |
| expect(screen.getByText('Continue')).toBeInTheDocument() | |
| expect(screen.getByText('Amount below 1 satoshi')).toBeInTheDocument() | |
| // validation message should appear for amounts below 1 satoshi | |
| expect(screen.getByText('Amount below 1 satoshi')).toBeInTheDocument() |
🤖 Prompt for AI Agents
In src/test/screens/wallet/send.test.tsx around lines 82-83, the inline comment
"button continue should be present" is outdated; update it to accurately reflect
that the test asserts the validation message/label for amounts below 1 satoshi.
Replace the comment with a short note such as "button shows 'Amount below 1
satoshi' for satoshis < 1" (or similar) so the comment matches the
expect(screen.getByText('Amount below 1 satoshi')).toBeInTheDocument()
assertion.
Summary by CodeRabbit
Chores
Tests