feat: target to rgb-lib 0.3.0-beta.5 - #1
Open
shashank-shinde wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughThis pull request updates the RGB library dependency from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/Wallet.ts (2)
926-964:⚠️ Potential issue | 🟠 MajorDon’t insert
expirationTimestampbefore the existingskipSyncargument.This changes the meaning of existing
wallet.send(..., true)call sites. The sixth argument used to beskipSync; after this change it becomesexpirationTimestamp, so older JS consumers will either hit a native type mismatch or send with an unintended expiry while still syncing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Wallet.ts` around lines 926 - 964, The new parameter order broke existing call sites by inserting expirationTimestamp before skipSync; restore backward compatibility by moving skipSync back to the original position (before expirationTimestamp) in the Wallet.send signature (and in the call into Rgb.send), i.e., use async send(recipientMap, donation, feeRate, minConfirmations, skipSync: boolean = false, expirationTimestamp: number = 0) and pass arguments to Rgb.send in the same order (this.getWalletId(), recipientMap, donation, feeRate, minConfirmations, skipSync, expirationTimestamp) so existing callers’ sixth argument remains skipSync; update the parameter defaults accordingly.
725-760:⚠️ Potential issue | 🟠 MajorRemoving
replaceRightsNumhere is a source-breaking API change.Extra positional args are still accepted in JS, so old
wallet.issueAssetIfa(..., replaceRightsNum, rejectListUrl)call sites will now bind the formerreplaceRightsNumtorejectListUrland silently drop the real URL. Keep a deprecated overload/shim for one release, or ship this as an explicit breaking change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Wallet.ts` around lines 725 - 760, The public method issueAssetIfa was changed to drop the old positional parameter replaceRightsNum which is source-breaking; restore a deprecated shim/overload in the Wallet.issueAssetIfa implementation that detects when the fifth argument is a number (replaceRightsNum) and the sixth argument (rejectListUrl) is omitted or mis-bound, then translate the call to the new signature by passing null or the correct rejectListUrl and handling/deprecating replaceRightsNum (e.g., log or ignore) before delegating to Rgb.issueAssetIfa(this.getWalletId(), ...); keep the new strict parameter order moving forward but accept the legacy form for one release to avoid silently mis-binding replaceRightsNum into rejectListUrl.android/src/main/java/com/rgb/RgbModule.kt (1)
199-207:⚠️ Potential issue | 🟠 MajorAndroid
SIGNET_CUSTOMsupport is only partially wired up.
getNetwork()now acceptsSIGNET_CUSTOM, butgenerateKeys,restoreKeys, and thegetWalletDatanetwork switch still only handleSIGNET. After this PR, Android can create that network in some paths and still reject it in others or throw when serializing wallet data back to JS.Also applies to: 1096-1129
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/src/main/java/com/rgb/RgbModule.kt` around lines 199 - 207, getNetwork() now recognizes "SIGNET_CUSTOM" but other places still only handle "SIGNET", causing inconsistent behavior; update the network handling in generateKeys, restoreKeys, and getWalletData to also accept and map "SIGNET_CUSTOM" to BitcoinNetwork.SIGNET_CUSTOM (and any related enum checks) so the same network variant is used across all paths and wallet serialization to JS; search for switch/when blocks or string comparisons in the functions generateKeys, restoreKeys, and getWalletData and add the SIGNET_CUSTOM branch (mirroring the SIGNET handling) so the network is consistently wired end-to-end.ios/Rgb.swift (1)
1070-1076:⚠️ Potential issue | 🔴 Critical
SIGNET_CUSTOMis still unsupported on inbound iOS paths.These serializers now emit
SIGNET_CUSTOM, butgetNetwork(_:),_generateKeys, and_restoreKeysabove still fall through tofatalErrorfor that value. Initializing or regenerating keys for a custom signet wallet will hard-crash the app.Also applies to: 2058-2065
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Rgb.swift` around lines 1070 - 1076, The serializers now emit .signetCustom but getNetwork(_:), _generateKeys, and _restoreKeys still fall through to fatalError for that enum case; update those functions to handle .signetCustom instead of crashing—either map .signetCustom to the same internal behavior used for .signet (if custom parameters are not required) or read the custom signet parameters from the wallet data and initialize the network/keys accordingly; replace the fatalError branches in getNetwork(_:), _generateKeys, and _restoreKeys with the appropriate handling so initializing/regenerating keys for a SIGNET_CUSTOM wallet no longer hard-crashes.
🧹 Nitpick comments (2)
src/NativeRgb.ts (2)
405-406: Avoid leaking the0"unset" sentinel into the typed API.The receive-side APIs model missing expiries as
null, butsend/sendBeginnow require anumberand rely on a comment telling callers to pass0. Usingnumber | nullhere and normalizing to0at the wrapper/native boundary would keep the contract symmetric and easier to call correctly.♻️ Suggested typing change
feeRate: number, minConfirmations: number, - expirationTimestamp: number, + expirationTimestamp: number | null, skipSync: booleanfeeRate: number, minConfirmations: number, - expirationTimestamp: number, + expirationTimestamp: number | null, dryRun: booleanAlso applies to: 430-432
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NativeRgb.ts` around lines 405 - 406, Change the public API types for expirationTimestamp from a plain number to number | null so callers can express "unset" with null; update all type declarations that currently use expirationTimestamp: number (including the other occurrences around the same area) to expirationTimestamp: number | null, and then normalize to 0 at the boundary where we call the native bridge (inside the send/sendBegin wrappers) by mapping null -> 0 before constructing the native payload. Ensure only the wrapper/native conversion uses the 0 sentinel and the rest of the codebase and callers work with null to represent missing expiries.
14-20: Centralize these literal unions instead of repeating them inline.This PR had to update the same
bitcoinNetworkandassignment.typeliterals in six places. A shared alias would make the next enum change much less drift-prone.♻️ Example direction
+type BitcoinNetwork = + | 'MAINNET' + | 'TESTNET' + | 'TESTNET4' + | 'REGTEST' + | 'SIGNET' + | 'SIGNET_CUSTOM'; + +type AssignmentType = + | 'FUNGIBLE' + | 'NON_FUNGIBLE' + | 'INFLATION_RIGHT' + | 'ANY';Then reuse
BitcoinNetwork/AssignmentTypein these method signatures instead of inlining the literals each time.Also applies to: 29-35, 82-82, 396-396, 422-422, 467-467
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NativeRgb.ts` around lines 14 - 20, Create central type aliases for the repeated literal unions (e.g., define type BitcoinNetwork = 'MAINNET' | 'TESTNET' | 'TESTNET4' | 'REGTEST' | 'SIGNET' | 'SIGNET_CUSTOM' and type AssignmentType = /* the assignment.type literals seen inline */) near the top of src/NativeRgb.ts and then replace every inline occurrence with these aliases (references include the bitcoinNetwork unions and the assignment.type union usages found around lines ~14-20, ~29-35, ~82, ~396, ~422, ~467). Update function/method signatures and any interfaces or type annotations (e.g., parameters, return types, and object property types) to use BitcoinNetwork and AssignmentType so all six places share the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/Rgb.swift`:
- Around line 621-627: The code is passing durationSeconds as an absolute expiry
into wallet.blindReceive (and other wallet.*Receive calls) instead of a relative
duration; change the call sites (e.g., the wallet.blindReceive invocation and
the other apply at lines ~2006-2012) to compute an absolute expirationTimestamp
by adding the current Unix time to durationSeconds (e.g., let expiration =
durationSeconds != nil ? UInt64(Date().timeIntervalSince1970) +
durationSeconds!.uint64Value : nil) and pass that expiration value into
expirationTimestamp, and make the same change in the Android bridge so both
platforms treat the JS API’s duration as a relative offset.
---
Outside diff comments:
In `@android/src/main/java/com/rgb/RgbModule.kt`:
- Around line 199-207: getNetwork() now recognizes "SIGNET_CUSTOM" but other
places still only handle "SIGNET", causing inconsistent behavior; update the
network handling in generateKeys, restoreKeys, and getWalletData to also accept
and map "SIGNET_CUSTOM" to BitcoinNetwork.SIGNET_CUSTOM (and any related enum
checks) so the same network variant is used across all paths and wallet
serialization to JS; search for switch/when blocks or string comparisons in the
functions generateKeys, restoreKeys, and getWalletData and add the SIGNET_CUSTOM
branch (mirroring the SIGNET handling) so the network is consistently wired
end-to-end.
In `@ios/Rgb.swift`:
- Around line 1070-1076: The serializers now emit .signetCustom but
getNetwork(_:), _generateKeys, and _restoreKeys still fall through to fatalError
for that enum case; update those functions to handle .signetCustom instead of
crashing—either map .signetCustom to the same internal behavior used for .signet
(if custom parameters are not required) or read the custom signet parameters
from the wallet data and initialize the network/keys accordingly; replace the
fatalError branches in getNetwork(_:), _generateKeys, and _restoreKeys with the
appropriate handling so initializing/regenerating keys for a SIGNET_CUSTOM
wallet no longer hard-crashes.
In `@src/Wallet.ts`:
- Around line 926-964: The new parameter order broke existing call sites by
inserting expirationTimestamp before skipSync; restore backward compatibility by
moving skipSync back to the original position (before expirationTimestamp) in
the Wallet.send signature (and in the call into Rgb.send), i.e., use async
send(recipientMap, donation, feeRate, minConfirmations, skipSync: boolean =
false, expirationTimestamp: number = 0) and pass arguments to Rgb.send in the
same order (this.getWalletId(), recipientMap, donation, feeRate,
minConfirmations, skipSync, expirationTimestamp) so existing callers’ sixth
argument remains skipSync; update the parameter defaults accordingly.
- Around line 725-760: The public method issueAssetIfa was changed to drop the
old positional parameter replaceRightsNum which is source-breaking; restore a
deprecated shim/overload in the Wallet.issueAssetIfa implementation that detects
when the fifth argument is a number (replaceRightsNum) and the sixth argument
(rejectListUrl) is omitted or mis-bound, then translate the call to the new
signature by passing null or the correct rejectListUrl and handling/deprecating
replaceRightsNum (e.g., log or ignore) before delegating to
Rgb.issueAssetIfa(this.getWalletId(), ...); keep the new strict parameter order
moving forward but accept the legacy form for one release to avoid silently
mis-binding replaceRightsNum into rejectListUrl.
---
Nitpick comments:
In `@src/NativeRgb.ts`:
- Around line 405-406: Change the public API types for expirationTimestamp from
a plain number to number | null so callers can express "unset" with null; update
all type declarations that currently use expirationTimestamp: number (including
the other occurrences around the same area) to expirationTimestamp: number |
null, and then normalize to 0 at the boundary where we call the native bridge
(inside the send/sendBegin wrappers) by mapping null -> 0 before constructing
the native payload. Ensure only the wrapper/native conversion uses the 0
sentinel and the rest of the codebase and callers work with null to represent
missing expiries.
- Around line 14-20: Create central type aliases for the repeated literal unions
(e.g., define type BitcoinNetwork = 'MAINNET' | 'TESTNET' | 'TESTNET4' |
'REGTEST' | 'SIGNET' | 'SIGNET_CUSTOM' and type AssignmentType = /* the
assignment.type literals seen inline */) near the top of src/NativeRgb.ts and
then replace every inline occurrence with these aliases (references include the
bitcoinNetwork unions and the assignment.type union usages found around lines
~14-20, ~29-35, ~82, ~396, ~422, ~467). Update function/method signatures and
any interfaces or type annotations (e.g., parameters, return types, and object
property types) to use BitcoinNetwork and AssignmentType so all six places share
the single source of truth.
🪄 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: 42e7ca46-375b-4eb7-8442-a2b016f28e10
📒 Files selected for processing (10)
android/build.gradleandroid/src/main/java/com/rgb/RgbModule.ktios/Rgb.mmios/Rgb.swiftios/RgbLib.swiftpackage.jsonscripts/download-rgb-lib-ios.jssrc/Interfaces.tssrc/NativeRgb.tssrc/Wallet.ts
Comment on lines
621
to
627
| let receiveData = try session.wallet.blindReceive( | ||
| assetId: assetId, | ||
| assignment: assignmentObj, | ||
| durationSeconds: durationSeconds?.uint32Value, | ||
| expirationTimestamp: durationSeconds?.uint64Value, | ||
| transportEndpoints: endpoints, | ||
| minConfirmations: minConfirmations.uint8Value | ||
| ) |
There was a problem hiding this comment.
durationSeconds is being forwarded as an absolute expiry.
The JS API still documents a relative duration, but both receive paths now pass that value into wallet.*Receive(... expirationTimestamp: ...). Passing 3600 will expire near Unix time 3600, not “one hour from now”. Convert to now + durationSeconds here, or rename/update the JS contract to take an absolute timestamp. Android mirrors the same conversion, so the fix needs to land in both bridges.
Also applies to: 2006-2012
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ios/Rgb.swift` around lines 621 - 627, The code is passing durationSeconds as
an absolute expiry into wallet.blindReceive (and other wallet.*Receive calls)
instead of a relative duration; change the call sites (e.g., the
wallet.blindReceive invocation and the other apply at lines ~2006-2012) to
compute an absolute expirationTimestamp by adding the current Unix time to
durationSeconds (e.g., let expiration = durationSeconds != nil ?
UInt64(Date().timeIntervalSince1970) + durationSeconds!.uint64Value : nil) and
pass that expiration value into expirationTimestamp, and make the same change in
the Android bridge so both platforms treat the JS API’s duration as a relative
offset.
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
1 participant
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.
Summary by CodeRabbit
New Features
getAssetMetadataAPI for enhanced asset informationdryRunmode for transaction previewBreaking Changes
replaceRightsNumparameter from asset issuanceexpirationfield toexpirationTimestampDependency Updates