feat: asset DIDs and public credentials#378
Conversation
weichweich
left a comment
There was a problem hiding this comment.
Some questions and some minor comments. But I also think the proxy filter is wrong? That needs to be changed.
Will give it another run after our call, but looks good initially. 🐝
nodes/parachain/src/rpc.rs
Outdated
| // Input subject ID | ||
| String, | ||
| // Runtime subject ID | ||
| runtime_common::assets::AssetDid, |
There was a problem hiding this comment.
Any reason for using the qualified path here and not simply AssetDid?
| Generic(GenericAssetId), | ||
| } | ||
|
|
||
| impl From<Slip44Reference> for AssetId { |
There was a problem hiding this comment.
The cherry 🍒 on top: I think there is a way to derive the from impl for each enum variant. You only implemented it for two because those are useful right now?
There was a problem hiding this comment.
Yeah good to know, but the problem is that not all references are unambiguous. For instance an EvmSmartContractNonFungibleReference could refer to either an Erc721 or an Erc1155 asset type.
crates/assets/src/asset.rs
Outdated
| (Some(SLIP44_NAMESPACE), Some(slip44_reference), identifier) => { | ||
| if identifier.is_some() { | ||
| log::trace!("Slip44 namespace does not accept an asset identifier."); | ||
| Err(Error::InvalidFormat) | ||
| } else { | ||
| Slip44Reference::from_utf8_encoded(slip44_reference).map(Self::Slip44) | ||
| } | ||
| } |
There was a problem hiding this comment.
I would separate them using a comment but use match through out the whole thing and not nest it with if
crates/assets/src/asset.rs
Outdated
| (Some(SLIP44_NAMESPACE), Some(slip44_reference), identifier) => { | ||
| if identifier.is_some() { | ||
| log::trace!("Slip44 namespace does not accept an asset identifier."); | ||
| Err(Error::InvalidFormat) | ||
| } else { | ||
| Slip44Reference::from_utf8_encoded(slip44_reference).map(Self::Slip44) | ||
| } | ||
| } |
There was a problem hiding this comment.
Alternativ: don't use if but something like 'ensure!()'
crates/assets/src/v1.rs
Outdated
| match (chain, asset) { | ||
| (Some(chain), Some(asset)) => { | ||
| let chain_id = ChainId::from_utf8_encoded(chain).map_err(AssetDidError::ChainId)?; | ||
| let asset_id = AssetId::from_utf8_encoded(asset).map_err(AssetDidError::AssetId)?; | ||
| Ok(Self { chain_id, asset_id }) | ||
| } | ||
| _ => Err(AssetDidError::InvalidFormat), | ||
| } |
There was a problem hiding this comment.
The easier extend is not an argument IMHO since the work to change a if-else to match is something any future dev can do. 😁 I also think we should stick with the linter rule.
pallets/attestation/Cargo.toml
Outdated
| name = "attestation" | ||
| repository = "https://github.com/KILTprotocol/kilt-node" | ||
| version = "1.7.1" | ||
| repository = "https://github.com/KILTprotocol/mashnet-node" |
There was a problem hiding this comment.
I suppose this is an inherent error from my update. There are many more. But we can handle this in a separate PR.
| repository = "https://github.com/KILTprotocol/mashnet-node" | |
| repository = "https://github.com/KILTprotocol/kilt-node" |
| sp-runtime = {git = "https://github.com/paritytech/substrate", default-features = false, branch = "polkadot-v0.9.28"} | ||
|
|
||
| # Internal runtime dependencies | ||
| public-credentials-runtime-api = {version = "1.7.2", path = "./runtime-api"} |
There was a problem hiding this comment.
| public-credentials-runtime-api = {version = "1.7.2", path = "./runtime-api"} | |
| public-credentials-runtime-api = {path = "./runtime-api"} |

fixes KILTProtocol/ticket#2029
This PR introduces the following components:
cratesfolder which contains additional crates we work on and might want to offer to the community. In this case, the hope is that the Asset DID crate would be migrated to the Substrate repo eventuallypublic-credentialspallet which stores credentials issued to assets as defined in the draftPallet
The new pallet exposes the following extrinsics:
EDIT: with the merge of #392, two new extrinsics will be added, and the
claim_hashis replaced by thecredential_id:The
InputCredentialtype is the following:EDIT: with the merge of #392, the format will be changed to:
RPC
The RPC exposes the following functions:
EDIT: with the merge of #392, the RPC endpoints will be:
For update examples see the description of #392.