feat: assets #31
feat: assets #31
Conversation
…universalprofile.cloud into chore/setup-DEV-6010
…ofile.cloud into feat/connect-profile-DEV-6319
…ofile.cloud into feat/lsp7-assets-DEV-5750
Deployed with Cloudflare Pages ☁️ 🚀 🆗 |
| const { tokens, assetFilter } = storeToRefs(useProfileStore()) | ||
| const lyxAsset = computed<Token>(() => { | ||
| return { | ||
| address: '0x0', |
There was a problem hiding this comment.
Why 0x0 over undefined or null?
There was a problem hiding this comment.
When i try to use it from the preview URL i see this, might be related?
There was a problem hiding this comment.
Ah now my bad - i think it is a version issue,
But maybe still, the 0x0 default / init state should not log an error in the dev console imo
There was a problem hiding this comment.
address field is required here but since we don't really show it for LYX it's 0x0.
composables/useErc725.ts
Outdated
| const config = { | ||
| ipfsGateway: network.ipfsUrl, | ||
| ipfsGateway: currentNetwork.ipfsUrl, |
There was a problem hiding this comment.
why is IPFS related to the network? It should not?
We use the same IPFS for any L14, L16, testnet, mainnet etc.
There was a problem hiding this comment.
I don't think it is needed to put the ipfsUrl in the network settings. It is not related.
There was a problem hiding this comment.
Alright I will change this. I was thinking that we might want to use different storage per network but I guess I was wrong.
| import { LSP3Profile } from '@lukso/lsp-factory.js' | ||
| import Web3 from 'web3' | ||
| import { LSP4DigitalAssetJSON } from '@lukso/lsp-factory.js/build/main/src/lib/interfaces/lsp4-digital-asset' |
There was a problem hiding this comment.
Im not sure we want to have a "link" here to lsp-factory.js
Im not sure if it is lsp-factory.js 's responsibility to provide these interfaces.
It is very handy, but maybe these interfaces could have a better "home" than lsp-factory.js
@CallumGrindle what do you think?
There was a problem hiding this comment.
yeah these typings are handy but it seems overkill to have to import the whole lsp-factory just for metadata types. I think we can move the types + the LSP6 methods + schemas from erc725.js into some kind of lsp utils lib. There isnt loads but there starts to be enough for its own lib
There was a problem hiding this comment.
idd we use package just for types here. I guess we should have npm package with just typings maybe
composables/useErc725.ts
Outdated
| const result = await erc725.fetchData(schema) | ||
| const assetAddresses = result.value as Address[] | ||
| const { profile } = useProfileStore() | ||
| console.log(schema, assetAddresses) |
There was a problem hiding this comment.
do you want to keep this console log?
There was a problem hiding this comment.
Good catch, it was just for debugging
composables/useErc725.ts
Outdated
| if (tokenIdType === TokenIdType.address) { | ||
| return lsp8MetadataGetter( | ||
| 'address', | ||
| // ethers.utils.hexDataSlice(tokenId.toString(), 12) | ||
| tokenId.toString() | ||
| ) | ||
| } else if (tokenIdType === TokenIdType.number) { | ||
| return lsp8MetadataGetter('uint256', parseInt(tokenId).toString()) | ||
| } else if (tokenIdType === TokenIdType.bytes32) { | ||
| return lsp8MetadataGetter('bytes32', tokenId.toString()) | ||
| } else { |
There was a problem hiding this comment.
it looks like a good candidate for a switch case on the tokenIdType variable no?
| @@ -0,0 +1,51 @@ | |||
| [ | |||
There was a problem hiding this comment.
it makes me think wheter or not we should also put this schema here:
https://github.com/ERC725Alliance/erc725.js/tree/develop/schemas
I think yes bc why not?
@CallumGrindle what do you think?
There was a problem hiding this comment.
I mentioned this a while ago and created ticket as well https://app.clickup.com/t/2645698/DEV-5997
| export const detectStandard = async ( | ||
| contractAddress: Address | ||
| ): Promise<InterfaceId | undefined> => { |
There was a problem hiding this comment.
Can this be of any help?
https://docs.lukso.tech/tools/erc725js/classes/ERC725#supportsinterface
There was a problem hiding this comment.
Sure, I didn't know about this function!
Ticket ID
DEV-5750
DEV-5751
DEV-5753
DEV-5754
Description
Loading LSP7 and LSP8 assets on wallet dashboard.