feat: add Solana support #62
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
Swapping from EVM to Solana: 
|
|
Swapping from Solana to EVM: 
|
|
To test, clone zeta-chain/example-contracts#232 and run: It should have this version of localnet pre-installed (as 6.0.0-rc3). |
| @@ -28,5 +28,8 @@ jobs: | |||
| - name: Install Dependencies | |||
| run: yarn | |||
|
|
|||
| - name: Build | |||
| run: yarn build | |||
There was a problem hiding this comment.
do we need above build workflow as well?
There was a problem hiding this comment.
For now, yes, because the script uses gateway.so by resolving @zetachain/localnet, and the build step is needed so that the file is copied from src into dist, so that the script can find it. Once we have these files published on npm, this step will no longer be needed.
| import { deployOpts } from "./deployOpts"; | ||
| import * as TestERC20 from "@zetachain/protocol-contracts/abi/TestERC20.sol/TestERC20.json"; | ||
|
|
||
| export const createToken = async ( |
There was a problem hiding this comment.
createToken seems a bit too generic and confusing what it refers to (and also it is in generic place), especially since localnet already have multiple chains and that will increase
There was a problem hiding this comment.
I suppose you're right, we can refactor this into something more manageable. I just needed a function that would mint ZRC-20, ERC-20 (if needed), created a pool with ZETA, etc. Adding SUI/ZRC-20 SUI was actually very easy thanks to this function, it was just one line of code.
packages/localnet/src/evmCall.ts
Outdated
| if (exitOnError) { | ||
| throw new Error(err); | ||
| } | ||
| logErr("ZetaChain", `Error executing onCall: ${err}`); |
There was a problem hiding this comment.
nit, but might be useful to log even if exitOnError is true
There was a problem hiding this comment.
I think the error gets logged inside the if statement. The logErr just provides nicer output.
| foreignCoins: any[]; | ||
| tss: any; | ||
| }) => { | ||
| const zrc20 = args[3]; |
There was a problem hiding this comment.
this might be subjective, but i think arrays can be deconstructed in js, so something like [_, receiver, zrc20, amount] = args might be cleaner
There was a problem hiding this comment.
Agree! I'll refactor the code, thanks!
| tss: any; | ||
| }) => { | ||
| const zrc20 = args[3]; | ||
| const chainID = foreignCoins.find( |
There was a problem hiding this comment.
i think check for chainID should be here as well if it can be undefined based on ?. operator
packages/localnet/src/solanaSetup.ts
Outdated
| `Monitoring new transactions for program: ${gatewayProgram.programId.toBase58()}` | ||
| ); | ||
|
|
||
| let lastSignature: string | undefined = undefined; |
There was a problem hiding this comment.
this can be simplified, i dont think undefined parts are needed
packages/localnet/src/solanaSetup.ts
Outdated
| ); | ||
|
|
||
| if (transaction) { | ||
| // console.log( |
There was a problem hiding this comment.
nit: we can remove commented code if not used
| } finally { | ||
| // Update lastSignature even if an error occurs | ||
| if (signatures && signatures.length > 0) { | ||
| lastSignature = signatures[0].signature; |
There was a problem hiding this comment.
is last signature on index 0?
There was a problem hiding this comment.
I believe so, they're returned in descending order so the latest one should be at 0.
| handlers.depositAndCall(args); | ||
| } else if (decodedInstruction.name === "deposit") { | ||
| handlers.deposit(args); | ||
| } |
There was a problem hiding this comment.
up to you, but this can probably be refactored a bit, seems like a lot of things are going on, even visually based on amount of {...} brackets, but not blocker for this PR
|
|
||
| const context = { | ||
| chainID, | ||
| origin: chainID === "901" ? sender : ethers.ZeroAddress, |
There was a problem hiding this comment.
I think so, because we need to set context.sender for EVM chains and context.origin for Solana.
|
@skosito thanks for the review! Addressed most of the comments, some improvements will be made in follow-up PRs. |
solanaCLI is installedWorks with zeta-chain/example-contracts#232
Refactor
https://www.figma.com/board/OIkQtFPbsSc9FgZaf6nmoO/Localnet-Architecture?node-id=0-1&t=xvUDq8lw5PkzB5t1-1