-
Notifications
You must be signed in to change notification settings - Fork 55
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
Introduce AssetUploader
module and integrate Irys
as an asset uploader provider
#318
Conversation
c5f6782
to
7aab4c4
Compare
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.
overall lgtm! one question i have:
RE if clients manually installed irys sdk is lower than what we expected, in this case, it should break instead of warning? cuz older irys version might not support sdk v2 at all
src/api/aptosConfig.ts
Outdated
/** | ||
* Asset uploader provider, default to "irys" | ||
*/ | ||
readonly assetUploaderProvider = "irys"; |
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.
lets make this a enum?
This a default npm process, if a package declares a peer dependency at version X and the host package has a lower version than X, npm would show this warning. I am not sure if/how we can detect the host package dependency version. Also, Irys released a patch version with the new Aptos SDK support so it should be pretty automatically for users to have the new version. A section on the SDK documentation page will be added for the |
7aab4c4
to
41f59db
Compare
const aptosConfig = new AptosConfig({ | ||
network: Network.TESTNET, | ||
}); |
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.
I'm guessing it only supports testnet?
src/plugins/assetUploader/index.ts
Outdated
|
||
static async init(config: AptosConfig) { | ||
switch (config.assetUploaderProvider) { | ||
case "irys": { |
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.
Enum? :)
src/plugins/assetUploader/irys.ts
Outdated
case Network.DEVNET: | ||
// Irys does not support Aptos' devnet | ||
throw new Error("Irys does not support Aptos' devnet"); | ||
case Network.TESTNET: | ||
return { irysNode: "devnet", providerUrl: Network.TESTNET }; | ||
case Network.MAINNET: | ||
// Irys supports node1 and node2, there is no major difference between | ||
// those and it is recommended to simply use node1 | ||
return { irysNode: "node1", providerUrl: Network.MAINNET }; | ||
default: | ||
throw new Error("Unsupported network"); |
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.
Could also just say Irys does not support ${network}
including devnet, but this owrks as well.
fbc5fff
to
950e622
Compare
950e622
to
ff953db
Compare
ff953db
to
598513e
Compare
…oader provider (#318) * integrate irys as an asset uploader service * address comments * upgrade irys package version * add known limitations of irys comments * run check version only on prs with main as branch target
…oader provider (#318) * integrate irys as an asset uploader service * address comments * upgrade irys package version * add known limitations of irys comments * run check version only on prs with main as branch target
Description
This PR introduces a generic
AssetUploader
module and Irys asset uploader provider as an optional dependency.We integrate
irys
package as a peer (optional) dependency to not add too much noise and maintenance for SDK users by forcing them to have a core dependency for a functionality they might not need.For users who want to use the SDK asset uploader, and more specifically Irys, they should install the
@irys/sdk
package manually.The SDK knows how to resolve whether the host package has the Irys package or not. If it doesnt and it tries to use Irys, the SDK would let the user know they should install the Irys package.
If the host package has Irys dependency with a lower version than what the SDK expects, it would warn the user that they have an unmet peer dependency
Test Plan
Related Links