fix(standards): nft enumeration & storage measurement #843
Conversation
…on and Approval Extension are not implemented
hanakannzashi
requested review from
austinabell,
ChaoticTempest and
itegulov
as code owners
hanakannzashi
changed the title
| .as_ref() | ||
| .and_then(|token_metadata_by_id| token_metadata_by_id.get(&token_id)); | ||
| let approved_account_ids = self.approvals_by_id.as_ref().and_then(|approvals_by_id| { | ||
| approvals_by_id.get(&token_id.to_string()).or_else(|| Some(HashMap::new())) |
There was a problem hiding this comment.
self.approvals_by_id.as_ref().then(|a| a.get(&token_id));
I think that if approval standard isn't implemented this should be None on the token. @BenKurrek can you confirm?
There was a problem hiding this comment.
If the approval management or metadata standards are not implemented, the token object that is serialized and sent back through the enumeration methods should include just the information as outlined in the core standard:
// The base structure that will be returned for a token. If contract is using
// extensions such as Approval Management, Metadata, or other
// attributes may be included in this structure.
type Token = {
token_id: string,
owner_id: string,
}
The approval and metadata standards are just additive.
There was a problem hiding this comment.
well, that's not how the standards are currently returning the data, it would be a breaking change to skip serializing if None right now, but can come in a standards re-write.
|
@BenKurrek do you mind having another quick look to make sure things are correct? Need to make sure this is correct when translating to JS SDK. cc @ailisp was there anything you think should be changed to be in sync with the JS NFT impl? |
|
@austinabell I don't have any so far, this specific PR looks good |
|
going to wait for another approval since I was the last to make a change |
austinabell
changed the title
In case of panic when TokenMetadata Extension and Approval Extension are not implemented