Refactor for full multichain support (SUI, Solana, Sonic SVM, Soon SVM, Eclipse SVM)#202
Refactor for full multichain support (SUI, Solana, Sonic SVM, Soon SVM, Eclipse SVM)#202devloai[bot] wants to merge 9 commits into
Conversation
Release 16 04 2024
Release 16 04 2024
Release 11 05 2024
Develop []
fix net worth
fix cetus coin normalization for 0x0
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Reviewer's Guide by SourceryThis pull request introduces a multichain architecture to the bot, enabling support for multiple blockchains beyond SUI. It includes a chain abstraction layer, chain configuration system, session data updates, a chain selection UI, a SUI provider implementation, placeholder implementations for other chains, and migration support for existing data. Sequence diagram for Chain SelectionsequenceDiagram
participant User
participant Bot
participant Session
User->>Bot: Selects 'Chain Selection' in settings
Bot->>Bot: Displays chain selection menu
Bot-->>User: Chain selection options (SUI, Solana, Ethereum)
User->>Bot: Selects a chain (e.g., Solana)
Bot->>Session: Updates session.selectedChain with 'solana'
alt session.wallets['solana'] is null
Bot->>Session: Generates new Solana wallet
Session-->>Bot: Solana wallet details
Bot->>Session: Stores Solana wallet in session.wallets['solana']
end
Bot-->>User: Confirmation message with selected chain
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
ok, other agent, farmer one, please review this |
|
No operation ID found for this PR |
|
what is operation ID? |
|
No operation ID found for this PR |
|
you mean this? |
|
No operation ID found for this PR |
|
@devloai what is operation ID? help him to find it pls |
|
Unable to address comment. You have run out of credits 😔 |
|
No operation ID found for this PR |
|
@sourcery-ai what is operation ID? |
|
No operation ID found for this PR |
|
It seems like However, the concept of an "operation ID" isn't a standard term in GitHub or pull request reviews. It's possible that Here's what I can suggest to help clarify the situation:
In the meantime, from my perspective as Sourcery, this pull request adequately addresses the requirements outlined in issue #201. The changes introduce a multi-chain architecture, which is the core objective. The bot's inability to find an "operation ID" doesn't necessarily indicate a problem with the code itself, but rather a potential issue with the integration of external tools or workflows. |
|
@sourcery-ai ok can you then please review this MR and do fixes yourself? im not reading your long reports, just do the job pls and lmk about effeciency |
|
No operation ID found for this PR |
|
agents can you fix oks operation ID finally or whats up |
|
No operation ID found for this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive multichain architecture to support SUI, Solana, and several SVM-based chains. The implementation introduces an abstraction layer with interfaces for wallet, transaction, asset, and pool operations across different blockchains.
Key Changes:
- Introduces chain abstraction layer with provider pattern and interfaces for cross-chain operations
- Updates session data structure to support multiple chain wallets with backward compatibility for existing SUI users
- Adds chain selection UI in settings menu with conversation flow for switching between chains
Reviewed Changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 20 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/index.ts | Updates SessionData to support multichain wallets with per-chain asset tracking |
| src/migrations/addMultichainSupport.ts | Migrates existing SUI-only sessions to new multichain structure |
| src/chains/config.ts | Defines chain configurations for all supported networks |
| src/chains/chain-utils.ts | Provides utility functions for chain provider management and wallet access |
| src/chains/interfaces/*.ts | Defines common interfaces for chain operations (IWallet, ITransaction, IAsset, IPool, IChainProvider) |
| src/chains/providers/sui/*.ts | Implements SUI provider using existing SDK for full chain support |
| src/chains/providers/solana/*.ts | Adds placeholder Solana providers with "coming soon" messages |
| src/chains/providers/index.ts | Factory function for retrieving chain providers |
| src/chains/home.ts | Routes home screen display based on selected chain |
| src/chains/settings/chain/*.ts | Implements chain selection UI and conversation flow |
| src/inline-keyboards/chain-selection.ts | Creates keyboard for chain selection interface |
| src/middleware/callbackQueries.ts | Adds handlers for chain selection callbacks |
| src/menu/settings.ts | Adds chain selection option to settings menu |
| src/index.ts | Updates session initialization to support multichain structure |
| README.md | Documents multichain architecture and instructions for adding new chains |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| solana: null, // Will be initialized when user selects Solana chain | ||
| 'sonic-svm': null, | ||
| 'soon-svm': null, | ||
| 'eclipse-svm': null |
There was a problem hiding this comment.
Setting wallet values to null will cause type errors because the ChainWallet interface expects privateKey and publicKey to be strings, not nullable. The wallets field is defined as [chain in ChainType]?: ChainWallet, meaning the keys are optional, but if present, they must be valid ChainWallet objects. Either use undefined instead of null, or make the ChainWallet interface nullable.
| }, | |
| solana: null, // Will be initialized when user selects Solana chain | |
| 'sonic-svm': null, | |
| 'soon-svm': null, | |
| 'eclipse-svm': null | |
| } | |
| // Other chains will be initialized when user selects them |
| return; | ||
| } | ||
|
|
||
| const selectedChain = chainMatch[1] as ChainType; |
There was a problem hiding this comment.
Type assertion selectedChain as ChainType is unsafe because the input hasn't been validated against the actual ChainType enum values. The regex only validates the format (\w+), not whether it's a valid chain. This could lead to runtime errors when the selected chain is not in the ChainType enum.
| const selectedChain = chainMatch[1] as ChainType; | |
| const selectedChainRaw = chainMatch[1]; | |
| if (!Object.values(ChainType).includes(selectedChainRaw as ChainType)) { | |
| await response.answerCallbackQuery('Invalid chain selected'); | |
| return; | |
| } | |
| const selectedChain = selectedChainRaw as ChainType; |
| ctx.session.privateKey = ctx.session.wallets[selectedChain]!.privateKey; | ||
| ctx.session.publicKey = ctx.session.wallets[selectedChain]!.publicKey; |
There was a problem hiding this comment.
The non-null assertion ctx.session.wallets[selectedChain]!.privateKey is unsafe. Even though there's a check at line 61, TypeScript doesn't recognize it as a type guard for the non-null assertion operator. If the wallet doesn't exist (which could happen for chains other than SUI), this will throw a runtime error. Consider using optional chaining or a proper check before accessing the properties.
| ctx.session.privateKey = ctx.session.wallets[selectedChain]!.privateKey; | |
| ctx.session.publicKey = ctx.session.wallets[selectedChain]!.publicKey; | |
| ctx.session.privateKey = ctx.session.wallets[selectedChain]?.privateKey; | |
| ctx.session.publicKey = ctx.session.wallets[selectedChain]?.publicKey; |
| export async function generateWalletForChain(chainType: ChainType): Promise<{ privateKey: string; publicKey: string }> { | ||
| const provider = await getChainProvider(chainType); | ||
| const walletInfo = provider.getWalletProvider().generateWallet(); | ||
|
|
||
| return { | ||
| privateKey: walletInfo.privateKey, | ||
| publicKey: walletInfo.address, |
There was a problem hiding this comment.
The WalletInfo interface has an address field but the function returns an object with a publicKey field. This inconsistency will cause type errors. The SUI provider's generateWallet() likely returns {privateKey, publicKey}, but WalletInfo expects {privateKey, address}. Either update the interface or the return mapping.
| export async function generateWalletForChain(chainType: ChainType): Promise<{ privateKey: string; publicKey: string }> { | |
| const provider = await getChainProvider(chainType); | |
| const walletInfo = provider.getWalletProvider().generateWallet(); | |
| return { | |
| privateKey: walletInfo.privateKey, | |
| publicKey: walletInfo.address, | |
| export async function generateWalletForChain(chainType: ChainType): Promise<{ privateKey: string; address: string }> { | |
| const provider = await getChainProvider(chainType); | |
| const walletInfo = provider.getWalletProvider().generateWallet(); | |
| return { | |
| privateKey: walletInfo.privateKey, | |
| address: walletInfo.address, |
| } | ||
|
|
||
| // Migrate assets structure | ||
| if (ctx.session.assets && !('sui' in ctx.session.assets)) { |
There was a problem hiding this comment.
The migration uses .includes() to check for 'sui' in ctx.session.assets, but assets is defined as a mapped type [chain in ChainType]?: { ... }. The proper way to check would be using the in operator: 'sui' in ctx.session.assets. Using .includes() on an object will not work as expected.
| getSessionKey: (ctx) => { | ||
| // Use the user ID as the session key | ||
| return ctx.from?.id.toString(); | ||
| }, |
There was a problem hiding this comment.
The getSessionKey function is unnecessary and could cause issues. Grammy's session middleware uses ctx.from?.id as the default session key. By explicitly setting it here, you're not adding any value, and the implementation has a potential issue: if ctx.from?.id is undefined, the function will return "undefined" as a string, which could lead to session collisions.
| getSessionKey: (ctx) => { | |
| // Use the user ID as the session key | |
| return ctx.from?.id.toString(); | |
| }, |
| txHash: result.digest, | ||
| success, | ||
| error: success ? undefined : 'Transaction failed', | ||
| blockNumber: result.checkpoint, // SUI uses checkpoint instead of block number |
There was a problem hiding this comment.
The blockNumber field in TransactionResult is typed as number but SUI uses checkpoints (strings), not traditional block numbers. At line 56, result.checkpoint (which could be a string) is being assigned to a field expecting a number. This could cause type errors or data loss if checkpoints are not numeric.
| isValidTokenAmount, | ||
| SUI_DECIMALS |
There was a problem hiding this comment.
Unused import SUI_DECIMALS.
| isValidTokenAmount, | |
| SUI_DECIMALS | |
| isValidTokenAmount |
| import { | ||
| getSuiProvider, | ||
| WalletManagerSingleton, | ||
| AftermathSingleton, |
There was a problem hiding this comment.
Unused import AftermathSingleton.
| AftermathSingleton, |
| */ | ||
| import { ChainConfig } from '../../config'; | ||
| import { IWallet, WalletBalance, WalletInfo } from '../../interfaces/IWallet'; | ||
| import { IAsset } from '../../interfaces/IAsset'; |
There was a problem hiding this comment.
Unused import IAsset.
| import { IAsset } from '../../interfaces/IAsset'; |
Description
This PR implements a comprehensive architecture to support multiple blockchains in RINbot, extending beyond the current SUI implementation to include Solana, Sonic SVM, Soon SVM, and Eclipse SVM as requested in issue #201.
Key Components
1. Chain Abstraction Layer
2. Chain Configuration System
ChainTypeenum andChainConfiginterface3. Session Data Structure Updates
selectedChainfieldChainWalletinterface for chain-specific wallet data4. Chain Selection UI
5. SUI Provider Implementation
6. Placeholder Implementations
7. Migration Support
Follow-up suggestions
Fixes #201
Summary by Sourcery
Implements a multi-chain architecture, allowing the bot to support SUI, Solana, and other SVM-based chains. It introduces a chain abstraction layer, a chain configuration system, and updates the session data structure to manage multiple wallets. It also adds a chain selection UI to the settings menu.
New Features: