Skip to content
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

Granular Asset Minting #491

Merged
merged 21 commits into from Aug 7, 2023
Merged

Granular Asset Minting #491

merged 21 commits into from Aug 7, 2023

Conversation

SilentCicero
Copy link
Member

@SilentCicero SilentCicero commented Jun 5, 2023

Abstract

This PR introduces granular asset minting which would enable sub identifiers to be included at asset creation. These sub identifiers can include important metadata and be used to enable native level NFTs.

This adds:

  • A new asset ID construction (which incorporates the contract ID and the sub asset ID).
  • New receipt logs for minting and burning which are helpful for indexing.
  • Changes to the MINT and BURN opcodes.

Cons

  • This removes the nice feature that asset IDs are the contract ID.

Pros

  • Native level UTXO based NFTs.
  • Single contract multi-asset pools (useful for common AMM designs).

Implementation tickets/PRs:

src/vm/instruction_set.md Outdated Show resolved Hide resolved
src/vm/instruction_set.md Outdated Show resolved Hide resolved
@Dentosal
Copy link
Member

Dentosal commented Jun 5, 2023

Possible optimization would be to add a cheap way to get the default subasset, i.e. all zeroes. It would be quite unfortunate if ALOC+CLI instruction pair must be performed even when a contract doesn't use subassets. A possible downside with this approach is that it's possible to not support subassets in some contract, but I think that can be alleviated by having proper tools on Sway side.

I think we can merge this as-is anyway, and figure out what, if any, optimizations are needed when implementing this.

src/protocol/id/asset.md Outdated Show resolved Hide resolved
src/vm/instruction_set.md Outdated Show resolved Hide resolved
@dmihal
Copy link
Contributor

dmihal commented Jun 6, 2023

I was just looking over EIP-3525, which uses the word "slot" to represent the sub-IDs of these tokens.

Any thoughts on the word "slot" instead of "sub_id"?

@SilentCicero @SwayStar123

@SwayStar123
Copy link
Member

I was just looking over EIP-3525, which uses the word "slot" to represent the sub-IDs of these tokens.

Any thoughts on the word "slot" instead of "sub_id"?

@SilentCicero @SwayStar123

Slots and token_ids seem to be seperate concepts in erc3525, I dont think it would be accurate here. The original multi-token rfc that I wrote was inspired by erc1155.

I would prefer just identifier over sub_indentifier, as contractid and assetid are already distinct i dont think there would be any confusion here, but sub_indentifier is also fine

# Asset ID

The _asset ID_ (also called _asset hash_) of a asset is computed as
the [hash](../cryptographic_primitives.md#hashing) of the `CONTRACT_ID` and a 256-bit `SUB_IDENTIFIER`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to support the None sub-identifier? In this case, the contract still can mint AssetId == ContractId.

Of course, we can't use None, but we can use some magic value instead like zeroed sub-identifier or a maximum sub-identifier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the decision was to rip the band-aid off, and break the tie that AssetId == ContractID

It'll be more confusing if they're sometimes equal, and sometimes different.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't need backward compatibility, then okay=) But it means it is a super breaking change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it's a breaking change, but that's why it's important we get it in soon so teams can adjust

@SilentCicero
Copy link
Member Author

Possible optimization would be to add a cheap way to get the default subasset, i.e. all zeroes. It would be quite unfortunate if ALOC+CLI instruction pair must be performed even when a contract doesn't use subassets. A possible downside with this approach is that it's possible to not support subassets in some contract, but I think that can be alleviated by having proper tools on Sway side.

I think we can merge this as-is anyway, and figure out what, if any, optimizations are needed when implementing this.

Regarding this, there might be some clever ways we can tuck the pre-image in the transaction.

xgreenx
xgreenx previously approved these changes Jul 11, 2023
src/abi/receipts.md Outdated Show resolved Hide resolved
src/abi/receipts.md Outdated Show resolved Hide resolved
{
"type": "Mint",
"sub_id": "0x39150017c9e38e5e280432d546fae345d6ce6d8fe4710162c2e3a95a6faff051",
"contract_id": "0x39150017c9e38e5e280432d546fae345d6ce6d8fe4710162c2e3a95a6faff051",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this breaks our convention of using id for the contract like other receipts. Any particular reason we used contract_id instead of just id? cc @SilentCicero @dmihal

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's nice to avoid confusion, since this interaction has 3 different IDs (contract ID, asset ID & sub-ID)

If we want to stay consistent, would it make sense to change id to contract_id in the other receipts?

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
@Voxelot Voxelot merged commit b241dc9 into master Aug 7, 2023
4 checks passed
@Voxelot Voxelot deleted the SilentCicero-multi-token branch August 7, 2023 21:40
IGI-111 pushed a commit to FuelLabs/sway that referenced this pull request Aug 14, 2023
This pull request is an upgrade to a `fuel-core 0.20.1` release.

The biggest changes coming from this new release is a rework of how
AssetId is structured to support [granular asset
minting](FuelLabs/fuel-specs#491). This means a
contract can mint and burn multiple AssetId's, and the ContractId !=
AssetId.

---------

Co-authored-by: bitzoic <bitzoic.eth@gmail.com>
Co-authored-by: Cameron Carstens <54727135+bitzoic@users.noreply.github.com>
Co-authored-by: Sophie Dankel <47993817+sdankel@users.noreply.github.com>
Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: Sophie <sophiedankel@gmail.com>
@dmihal
Copy link
Contributor

dmihal commented Sep 8, 2023

Any thoughts on having MINT and BURN return the resulting asset ID? Would save some contracts from needing to do duplicate hashing to calculate the address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants