-
Notifications
You must be signed in to change notification settings - Fork 19
feat: integrate RNS support across transfer, batch-transfer, and balance commands #205
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
base: main
Are you sure you want to change the base?
Conversation
- Add resolve command to lookup .rsk domain names - Support forward resolution (name to address) - Support reverse resolution (address to name) - Add mainnet and testnet support - Handle edge cases with user-friendly messages
Hi @97woo , If you have any questions, just post them on the PR or send me an email and I’ll get back to you asap. Thanks |
- Create reusable RNS utility functions - Add isRNSDomain to detect RNS domains - Add resolveRNSToAddress for forward resolution - Add resolveAddressToRNS for reverse resolution - Add resolveToAddress to handle both addresses and domains
- Add --rns option to specify recipient as RNS domain - Integrate RNS resolution before transfer execution - Support both RBTC and ERC20 token transfers with RNS
- Add --rns flag to enable RNS domain resolution - Update FileTx type to accept both addresses and domains - Support RNS domains in interactive mode - Support RNS domains in file-based batch transfers - Automatically resolve domains during execution
- Add RNS usage examples for transfer command - Add RNS usage examples for batch-transfer command - Add RNS usage examples for balance command - Update command descriptions to mention RNS support - Provide JSON examples with RNS domains for batch transfers
- Replace placeholder domains with actually registered ones - Use testing.rsk and rifos.rsk which exist on both mainnet/testnet - Add testnet examples for balance command - Clarify that --rns flag is needed for batch transfers with domains
- Adapt RNS integration to new object parameter style - Keep upstream refactoring with BatchTransferCommandOptions - Integrate RNS resolution with new logging system - Maintain compatibility with isExternal mode
@ezequiel-rodriguez Thank you for the great feedback! I've
Quick Examples:# Transfer to RNS domain
rsk-cli transfer --rns testing.rsk --value 0.001
# Batch transfer with RNS domains
rsk-cli batch-transfer --rns --file batch.json
# Check balance of RNS domain
rsk-cli balance --rns testing.rsk
Key Points:
- Created reusable RNS helper module to avoid code duplication
- Works on both mainnet and testnet
- Backward compatible - no breaking changes
- Updated documentation with examples
All changes have been pushed and tested. The integration works
seamlessly with the existing codebase and the recent refactoring.
Please review and let me know if you need any adjustments! |
Hi Ezequiel,
Hope you’re well. I’m following up on my contribution to *rsksmart/rsk-cli*,
*PR #205* — “feat: integrate RNS support across transfer, batch-transfer,
and balance commands,” opened on *Jul 21, 2025 (KST)* and last updated
around *Aug 11, 2025*.
Per your feedback, I:
-
expanded the scope beyond a standalone resolve command,
-
added a reusable *rnsHelper*,
-
integrated *RNS* across the *transfer*, *batch-transfer*, and *balance*
commands,
-
and updated the *README* with verified examples—while keeping backward
compatibility.
Link:
• PR #205: #205
Happy to iterate quickly on any feedback.
Thank you!
2025년 8월 9일 (토) 오전 7:54, Ezequiel Martin Rodriguez ***@***.***>님이
작성:
… *ezequiel-rodriguez* left a comment (rsksmart/rsk-cli#205)
<#205 (comment)>
Hi @97woo <https://github.com/97woo> ,
I was reviewing your PR and the initiative to integrate RNS into the CLI —
actually, this was one of our goals for this quarter. Having the command
separately is fine, but to really make an impact we should also integrate
RNS into the transfer, transfer-batch, and balance commands.
Ideally, we could add a -rns parameter to those commands and then pass the
.rsk domain so it gets resolved.
If you have any questions, just post them on the PR or send me an email
and I’ll get back to you asap.
Thanks
—
Reply to this email directly, view it on GitHub
<#205 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AW6Q5OHGCIWGY7R5DQFZYFD3MUTCPAVCNFSM6AAAAACB7ZBYV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTCNRZGQ4DAOBWGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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.
Sorry for the late response, we were working on the CLI for improving it and for make it usable with external interactions as MCP servers, Please take as reference other modules like wallet or transfer and follow the same style, and please avoid the comments along all the code.
- Move RNS contract addresses to constants.ts - Add isExternal parameter support for MCP compatibility - Remove all code comments - Add address validation for reverse lookup - Use consistent parameter patterns with other commands
- Use helper functions (logMessage, logError, etc.) consistently - Apply spinner helper functions throughout - Maintain emoji usage consistent with original codebase - Ensure MCP compatibility with isExternal parameter
- Refactor rnsHelper.ts to use object parameters like other modules - Update all RNS function calls in bin/index.ts and batchTransfer.ts - Remove unnecessary type casting and unused functions - Ensure consistency with wallet/transfer module patterns
Hello @97woo , as you are interacting with RNS, please make sure you use the package rns-resolver in order to keep standard use of the tool (instead of create if from scratch). |
- Fixed dynamic import issue with @rsksmart/rns-resolver.js package - Removed all code comments as per PR review requirements - Updated isExternal parameter propagation in resolve command - Maintained consistency with wallet/transfer module patterns
- Remove top-level await, use lazy loading instead - Add proper address validation with isAddress() - Improve type safety by removing unnecessary 'any' types - Fix potential runtime errors with proper validation
Hi @scguaquetam, Thank you for the thorough review! I've addressed all the ✅ Completed Changes:
📝 Technical Notes:The async function getResolver() {
const RNSResolverModule = await
import("@rsksmart/rns-resolver.js");
return (RNSResolverModule as any).default.default ||
(RNSResolverModule as any).default;
} This is a known issue with the package's module structure and ✅ Testing:
The implementation is now consistent with the rest of the Happy to iterate quickly on any remaining feedback. Thank you! |
PR #205 Updates – Ready for Review
Hi @scguaquetam,
Thank you again for your detailed review on PR #205.
I’ve now addressed all of the requested changes:
- Integrated official @rsksmart/rns-resolver.js package
- Removed all inline comments for consistency
- Followed wallet/transfer module patterns
- Added conditional spinner/log handling for MCP compatibility
- Used ZERO_ADDRESS constant from constants.js
- Improved validation (isAddress) and type safety
✅ Tests confirm correct resolution on both mainnet and testnet
✅ Reverse resolution works where configured
✅ Clean TypeScript build and runtime verified
The implementation is now consistent with the rest of the CLI and ready for
MCP server integration.
Could you please review the changes at your convenience?
Happy to adjust further if needed.
Best regards,
Geonwoo (97woo)
2025년 8월 28일 (목) 오전 4:39, Sebas ***@***.***>님이 작성:
… *scguaquetam* left a comment (rsksmart/rsk-cli#205)
<#205 (comment)>
Hello @97woo <https://github.com/97woo> , as you are interacting with
RNS, please make sure you use the package rns-resolver
***@***.***/rns-resolver> in order to keep
standard use of the tool (instead of create if from scratch).
See this project
<https://github.com/rsksmart/airdrop-ui/blob/master/src/hooks/useConnectRNSDomain.ts>
for reference
—
Reply to this email directly, view it on GitHub
<#205 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AW6Q5OHAKYIN4LXD2FQGFIL3PYCPHAVCNFSM6AAAAACB7ZBYV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMRZGUYTOOBZGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
}); | ||
|
||
program | ||
.command("resolve <name>") |
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.
we shoould call it just resolve?
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.
Yes it's just resolve
Hello @97woo, thanks so much for your work, please take in account the comments I put recently and work on them, I think there are the last details to polish. |
- Add minimum 5 character validation based on RNS contract requirements - Prevent non-RNS domains (like .com, .txt) from being recognized - Based on FIFSRegistrarBase.sol: uint public minLength = 5
Hi @scguaquetam, I've addressed all your
The code is ready for another review. Thank |
Hello @97woo I was running some tests, and on the transfer command, I found an error, which cannot find the reason, but when trying to sent a transfer on testnet , using this: can you take a look on this error? |
When testing reverse resolution, I found this, just get the address from a .rsk domain, that works good, but when using the same address to get it in reverse, there is a format error (checksum error) it would be nice if can implement a checksum function to avoid this error. |
The code requests I made before are okay @97woo , so the code is well formatted okay now. Just take a look on these comments I make about the RNS resolution and we would be done. |
Hi @scguaquetam — I’ve addressed your latest feedback:
Ready for another review. |
Hello @97woo , the first issue (transfer -t --wallet sebas --rns jormel.rsk --value 0.000001) is working correct, thank you!! When I tested the reverse one, I have this behavior (looks like it is not capable to solve reverse mode), does it work for you? is this a library issue ? can you take a look on it? sorry for the inconvenience! |
Hi @scguaquetam — thanks for confirming transfer now works. On reverse resolution: it’s data‑dependent. Reverse resolution requires a reverse record set by the address owner. If it’s missing, “Address has no reverse record” is expected and not a checksum or library error. What I checked:
How to verify:
If you can share an address that already has a reverse record, I’ll verify it end‑to‑end. I can also add a small hint in the CLI output explaining this if you’d like. |
Summary
Added a new
resolve
command to the RSK CLI that enables users toresolve RNS (RIF Name Service) domain names.
UPDATE: Following feedback, also integrated RNS support into
transfer
,batch-transfer
, andbalance
commands.Features
RNS Resolve Command
RNS Integration in Core Commands
--rns
parameter for recipientdomains
--rns
flag for domainresolution
--rns
parameter for domain holderbalance checks
Usage
Resolve Command