-
Notifications
You must be signed in to change notification settings - Fork 162
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
feat: Add adapter for Solana #472
Conversation
🦋 Changeset detectedLatest commit: 05979b6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
有几个问题: Ethereum 和 Solana 的地址格式不同按理来说,应该通过 Web3Provider 提供全局默认配置,然后组件的 Props 可以重写配置 所以两种链地址格式互相冲突。 是否应该不要在组件库中辅助处理格式化(添加 0x 前缀),而是遵从用户自己提供的配置呢? 因为开发者自己在展示组件的时候,提供一个 prefix 是很简单的事情,不要组件库帮忙处理应该也可以。 与之类似的还有 CroptoPrice 组件的 decimal、symbol 等 获取 SNS(Solana 域名服务)需要引入另一个包具体在 QuickNode SNS |
地址不全 SNS 这个可以放到 optionalDep 来支持 https://docs.npmjs.com/cli/v10/configuring-npm/package-json#optionaldependencies 。不过要研究下怎么写,这个可以放到最后再搞吧,可以先不支持。等下一个版本再支持。 |
目前这有个编译错误,没看明白是什么导致的。有人有这个经验吗? |
本地执行下 |
packages/assets/src/chains.tsx
Outdated
@@ -77,7 +78,7 @@ export const Optimism: Chain = { | |||
}; | |||
|
|||
export const Avalanche: Chain = { | |||
id: ChainIds.Avalanche, | |||
id: ChainIdToken.fromValue(ChainIds.Avalanche), |
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.
这里这么改是为什么?
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.
简单来说:主要是想将基于 wagmi EVM 这种提供了 chain.id 的,和 Solana 这种没有 chain.id 的,封装为一个统一的对象,提供一种统一的根据 id 判断是否是同一个链的方法。
EVM 的链,有个 chain id,number 格式。Solana 那些没有这个值。
所以整了个包装类型。数据结构是:
class {
value: any;
__key: string;
contractor(value, __key) {
// 如果有 value 则 this.value 就是 value
// 如果没有提供,this.value 就是 this 本身
}
}
实际使用的时候:
- 如果有 id
const chainMetadata = { id: 1, name: 'foo' };
const idToken = ChainIdToken.fromValue(chainMetadata.id);
const chainAssets1 = { id: idToken, name: 'foo' };
const chainAssets2 = { id: idToken, name: 'foo2' };
// 判断的时候
chainAssets1.value === chainAssets2.value; // -> true
- 如果是 Solana 这种没有 id 的
const chainMetadata = { name: 'foo' };
const idToken = ChainIdToken.fromKey(chainMetadata.name);
const chainAssets1 = { id: idToken, name: 'foo' };
const chainAssets2 = { id: idToken, name: 'foo2' };
// 判断的时候
chainAssets1.value === chainAssets2.value; // -> true
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.
不太建议这么改,我们的核心还是 EVM 链,而 chainId
在以太坊生态算是比较公认的概念了,它总是作为一个十进制数字或者以 0x
开头的十六进制字符串;
这里我觉得可以沿用当前的设计,简单的做个兼容,避免和以太坊生态的 chainId 重复即可。
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.
感觉可以加一个 network
用来标明是什么链,默认是 Ethereum
,为 Solana
就认为是 solana。
能复现,不过没看明白详细原因,本地报错也是就那两行 -_- |
packages/assets/src/chains.tsx
Outdated
@@ -77,7 +78,7 @@ export const Optimism: Chain = { | |||
}; | |||
|
|||
export const Avalanche: Chain = { | |||
id: ChainIds.Avalanche, | |||
id: ChainIdToken.fromValue(ChainIds.Avalanche), |
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.
不太建议这么改,我们的核心还是 EVM 链,而 chainId
在以太坊生态算是比较公认的概念了,它总是作为一个十进制数字或者以 0x
开头的十六进制字符串;
这里我觉得可以沿用当前的设计,简单的做个兼容,避免和以太坊生态的 chainId 重复即可。
packages/solana/CHANGELOG.md
Outdated
@@ -0,0 +1 @@ | |||
# @ant-design/web3-solana |
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.
CHANGELOG
会在发布前自动生成,这里需要在分支上执行 changeset
来记录下变更;
}); | ||
}, [publicKey, connected]); | ||
|
||
const wallets: Wallet[] = useMemo(() => { |
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.
这层 useMemo 没有实际起作用,可以考虑去掉
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.
修复的问题点一下下面的 resolve conversation
packages/web3/package.json
Outdated
@@ -45,6 +45,8 @@ | |||
}, | |||
"devDependencies": { | |||
"@ant-design/web3-wagmi": "workspace:*", | |||
"@ant-design/web3-solana": "workspace:*", | |||
"@solana/wallet-adapter-wallets": "^0.19.24", |
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.
同上,依赖需要梳理下,是否不需要声明 @solana/wallet-adapter-wallets
CI 的报错是构建类型文件 |
我发现执行 |
暂时先把 demos 里面的代码删掉了,报错改变了。现在的 error 是 chainid 类型问题,我再改成之前的 number 应该就可以了。 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #472 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 96 102 +6
Lines 4377 4796 +419
Branches 453 506 +53
==========================================
+ Hits 4377 4796 +419 ☔ View full report in Codecov by Sentry. |
如果是 demo 报错的话估计是 dumi 配置的问题。可能是 dumi 的 alias 配置有问题。我看现在 dumi 的 alias 配置是自动读取文件夹内容生成的。你可以调试下看看是不是生成的逻辑有问题。https://github.com/ant-design/ant-design-web3/blob/main/.dumirc.ts#L7 |
可以先写下 API 文档,这样可以 review 下用法,看看用户测使用的接口上有没有问题。这个可能更重要。内部实现都好改,对外接口一旦定义确定了就不好改了。 |
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.
没什么大问题了,先合并一个版本发布 0.x,后面再单独提 PR 优化,然后再发 1.0
A lot of updates, so create a pr first.
Test cases and demos will be add later.