Create new index for tracking Asset metadata #2445
Conversation
maschad
requested review from
acerone85,
netrome,
AurelienFT,
rymnc and
rafal-ch
|
Thanks for all your help @AurelienFT ❤️ |
| @@ -2,6 +2,12 @@ scalar Address | |||
|
|
|||
| scalar AssetId | |||
|
|
|||
| type AssetInfoDetails { | |||
| contractId: HexString! | |||
There was a problem hiding this comment.
Not sure here, but maybe we want to use ContractId! here, as in other places.
|
|
||
| fn asset_info(&self, asset_id: &AssetId) -> StorageResult<Option<AssetDetails>>; | ||
|
|
||
| fn asset_exists(&self, asset_id: &AssetId) -> StorageResult<bool>; |
There was a problem hiding this comment.
Seems like asset_exists is not used, can we remove it?
| }, | ||
| }; | ||
|
|
||
| /// Contract info |
There was a problem hiding this comment.
| /// Contract info | |
| /// Asset info |
| /// Contract info | ||
| pub struct AssetsInfo; | ||
|
|
||
| pub type AssetDetails = (ContractId, Bytes32, u64); // (contract_id, sub_id, total_amount) |
There was a problem hiding this comment.
Can we use struct with named fields? Something similar to what we have here.
| db.storage::<AssetsInfo>() | ||
| .insert(&asset_id, &(*contract_id, **sub_id, current_count))?; | ||
| } | ||
| _ => {} |
There was a problem hiding this comment.
Please enumerate all variants explicitly to avoid surprises when new variant is added in the future.
| .map(|info| { | ||
| info.2 | ||
| .checked_add(*val) |
There was a problem hiding this comment.
| .map(|info| { | |
| info.2 | |
| .checked_add(*val) | |
| .map(|info| { | |
| let (_, _, count) = *info; | |
| count | |
| .checked_add(*val) |
May also affect the Receipt::Burn branch.
| let current_count = match db.storage::<AssetsInfo>().get(&asset_id) { | ||
| Ok(count) => count, | ||
| Err(_) => { | ||
| // If asset doesn't exist yet, create it with 0 count | ||
| db.storage::<AssetsInfo>() | ||
| .insert(&asset_id, &(*contract_id, **sub_id, 0))?; | ||
| Some(Cow::Owned((*contract_id, **sub_id, 0))) | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we shouldn't treat all storage errors as "value not found". Some of them should just be bubbled up.
Maybe something like this will work:
let current_count = match db.storage::<AssetsInfo>().get(&asset_id)? {
Some(count) => Some(count),
None => {
// If asset doesn't exist yet, create it with 0 count
db.storage::<AssetsInfo>()
.insert(&asset_id, &(*contract_id, **sub_id, 0))?;
Some(Cow::Owned((*contract_id, **sub_id, 0)))
}
}
If you agree, please note that now two branches always return Some, so maybe we can get rid of the follow-up map() as well.
| }) | ||
| .map(|info| { | ||
| info.2 | ||
| .checked_sub(*val) |
There was a problem hiding this comment.
If we go down to 0 on Burn should we remove the asset from the index? Unless we need to distinguish between "never minted" and "minted and totally burned" cases.
Anyway, currently the index may grow indefinitely.
Which also makes me think that I should consider this for the balances indexation I added recently :-)
cc: @xgreenx, wdyt?
| .map(|info| { | ||
| info.2 | ||
| .checked_add(*val) | ||
| .ok_or(anyhow::anyhow!("Asset count overflow")) |
There was a problem hiding this comment.
You may consider integrating this with the IndexationError available here: https://github.com/FuelLabs/fuel-core/blob/master/crates/fuel-core/src/graphql_api/indexation.rs#L28
It's a common type for all errors related to integration and it'll grow even more variants after the "coins to spend" indexation is merged. See here.
| .0; | ||
|
|
||
| // Then | ||
| // We should have the minted amount first |
There was a problem hiding this comment.
| // We should have the minted amount first | |
| // We should have the minted amount reduced by the burned amount |
Linked Issues/PRs
Closes Add indexation
AssetId -> (ContractId, SubId)#2425Related to Consider
assetsin check-account utility fuels-ts#3373Description
This introduces a new GraphQL endpoint that returns info about an asset.
The off-chain worker indexes
MintandBurnevents to store this metadata about anAssetIdChecklist
Before requesting review
After merging, notify other teams
[Add or remove entries as needed]