rmrk-team/evm

RMRK Solidity contracts

RMRK EVM Package

A set of Solidity smart contracts implementing RMRK modular NFTs and compatible extensions for them.

Smart contracts documentation as well as usage instructions and tutorials can be found in the RMRK EVM developer documentation

Usage

To use the RMRK legos and smart contracts contained in this repository, simply add them to your project:

yarn add @rmrk-team/evm-contracts

or

npm -i @rmrk-team/evm-contracts

Once the dependency is added to your project, simply import the smart contracts you wish to utilize into your own smart contract.

RMRK Legos

RMRK is a set of NFT standards that compose several NFT module primitives. Putting these modules together allows a user to create NFT systems of arbitrary complexity. So far we have created 6 modules as ERC proposals, all of which are ERC721 compatible. The first 5 are already standards, the 6th is still in Draft.

• MultiAsset: ERC-5773: Context-Dependent Multi-Asset Tokens
• Nestable: ERC-7401: Parent-Governed Non-Fungible Tokens Nesting
• Composable & Equippable: ERC-6220: Composable NFTs utilizing Equippable Parts
• Soulbound: ERC-6454: Minimal Transferable NFT detection interface
• Emotable: ERC-7409: Public Non-Fungible Tokens Emote Repository
• Dynamic Attributes: ERC-7508: Dynamic On-Chain Token Attributes Repository
• ERC20-Holder: ERC-7590: ERC-20 Holder Extension for NFTs

evm's Issues

Remove the redundant _ from the variable

evm/contracts/implementations/RMRKBaseStorageImpl.sol

Line 19 in 31b092c

constructor(string memory symbol_, string memory type__)

The type_ variable has two _ characters. This seems redundant and could be removed.

So in short:

type__ => type_

Should `SetEquippableToAll(uint64 indexed partId)` be emitted when `_addEquippableAddresses` etc?

https://github.com/rmrk-team/evm/blob/master/contracts/RMRK/catalog/RMRKCatalog.sol

I found that when _addEquippableAddresses, _setEquippableAddresses or _resetEquippableAddresses,

delete _isEquippableToAll[partId]

is excuted, but no SetEquippableToAll(uint64 indexed partId) emit.

If your indexer just listen to SetEquippableToAll(...) event to update the equippableToAll state of part, you won't get the real time data.

A friendly reminder that the small problem in LISENSE

https://github.com/rmrk-team/evm/blob/master/LICENSE#L152

The index here should be starting from 5.

Cannot install `rmrk-team/evm-contract`

Pls check the above image, tried installing with yarn & npm, but throwing Error for both the case.

Do we have any workaround with this?

Add Resource-replace param

When adding a new resource, an issuer should be able to specify the ID of an existing one which would get overwritten. This is necessary to be able to upgrade NFTs, change their parts, or even just do reveal mechanics of certain types.

This should be an extra parameter passed alongside the add resource call.

Some questions about `MultiResource_EIP` in `eip` branch

I have questions about why these two functions did not get modified by virtual?

https://github.com/rmrk-team/evm/blob/eip/contracts/MultiResource_EIP/MultiResourceToken.sol#L389
https://github.com/rmrk-team/evm/blob/eip/contracts/MultiResource_EIP/MultiResourceToken.sol#L410

If so, this would mean that every resource in a project that directly publishes the standard as NFT can be accepted or rejected by anyone at will.

`RMRKCatalog` has no way to set `metadataURI`

There's no _setMetadataURI function and the _metadataURI is private, so there's no way for me to inherit this contract and set _metadataURI.
I think you internal _metadataURI or give a _setMetadataURI, maybe better internaling _metadataURI which will be more flexible for 2nd develop.

https://github.com/rmrk-team/evm/blob/master/contracts/RMRK/catalog/RMRKCatalog.sol

maxSupply: possibility to mint unlimited number of NFTs

In rmrk-pallets and original kusama spec maxSupply 0 equals to infinite number of tokens allowed to mint.

Should `reclaimChild` need to check the child is not in `_pendingChildren` and `_children`?

Because when child is in pendingChildren or children, the reclaimChild can still be called and this will cause record issue.

Add transferable flag

Simple “transferable” flag to be set on the collection, bool, set on mint and not mutable later. Needed for gems and all the “soulbound” stuff.

In Kusama’s implementation we also have a modifier for temporary transferability, so that you can say, this becomes non transferable after X blocks, or it is transferable for only X blocks, but to not complicate things just the basic boolean would do.

Concept is explained here.

Moar Tests

The following is a list of methods in the public API that looked like they could use test coverage:

• General
  • approve
  • isApprovedOrOwner
  • getApproved
  • tokenURI
• Resources
  • getResourceObject
  • getResourceOverwrites
  • hashResource16
• Introspection
  • isRmrkCore
• Nesting
  • deleteChildFromChildren
  • setChild
• Transfers
  • transfer
  • transferFrom
• Royalties
  • getRoyaltyData
  • setRoyaltyData

`safeNestTransfer` is redundant

evm/contracts/RMRK/RMRKNesting.sol

Line 292 in d921674

function _safeNestTransfer(

The "safe" function is for preventing token wrongly sending to a common contract, the _nestTransfer has already been seen as one Nesting token be sent to another Nesting contract, it did contract interface check in internal code.

I think it's better to define a safeTransferNestingFrom function as I defined in PR before.
safeTransferFrom means "I wanna safe sending the token as a ERC721 to a common contract".
safeTransferNestingFrom means "I wanna safe sending the token as a Nesting to a common contract".

Duplicate check is needed in `addChild`

I think here lost duplicate check:

evm/contracts/RMRK/RMRKNesting.sol

Line 396 in d921674

The same child could be added unlimited times.

Catalog implementation with a mapping and method to get all partIds

Currently without the indexer or without quering all PartsAdded events, there's no way to get all partIds. It doesn't help also that partIds are not sequential.

It would be great to have an implementation of Catalog (and list it as recommended) which as a view function to get all part ids.

After `_unnestChild`, position of exchanged child(last child) in `_posInChildArray` is not updated

As I mentioned in title,
We have [A, B, C] in _children[1], after unnestChild(1, 0, address(0)), _children[1] will become [C, B], but _posInChildArray[CAddr][CId] was still 2.

Should revert when try to transfer NFT to itself

If try to transfer NFT to itself, should revert it.
Rightnow the code will cause an infinite loop when run ownerOf in _approve in _updateOwnerAndClearApprovals.

When overwrites, forget to delete the corresponding element in `activeResourcePriorities`

https://github.com/rmrk-team/evm/blob/dev/contracts/RMRK/multiresource/AbstractMultiResource.sol#L183

Equipping into an NFT with 2 resources of same base

We should perhaps modify the Equip logic such that it pays attention to which resource is is equipping into as well.

Will that be more direct and intuitive if set the uninitialized priority to `2^16 - 1` in `_acceptChild`?

2^16 - 1 = 65535
The client can easily handle this number and everywhere still make sense. For client there's no need to resolve the special 0 anymore.

Deploy Issue

I have below error when i run deploy command: npx hardhat run scripts/deploy.ts

No need to generate any newer typings.
HardhatError: HH700: Artifact for contract "Greeter" not found.
at Artifacts._handleWrongArtifactForContractName (/Users/Waqar/Downloads/evm-master 2/node_modules/hardhat/src/internal/artifacts.ts:478:11)
at Artifacts._getArtifactPathFromFiles (/Users/Waqar/Downloads/evm-master 2/node_modules/hardhat/src/internal/artifacts.ts:592:19)
at Artifacts._getArtifactPath (/Users/Waqar/Downloads/evm-master 2/node_modules/hardhat/src/internal/artifacts.ts:275:17)
at async Artifacts.readArtifact (/Users/Waqar/Downloads/evm-master 2/node_modules/hardhat/src/internal/artifacts.ts:58:26)
at async getContractFactory (/Users/Waqar/Downloads/evm-master 2/node_modules/@nomiclabs/hardhat-ethers/src/internal/helpers.ts:91:22)
at async main (/Users/Waqar/Downloads/evm-master 2/scripts/deploy.ts:17:19)

Plz help

unused function parameter

This makes my OCD really hard.
plz

Warning: Unused function parameter. Remove or comment out the variable name to silence this warning.
  --> @rmrk-team/evm-contracts/contracts/RMRK/extension/RMRKRoyalties.sol:76:9:
   |
76 |         uint256 tokenId,
   |         ^^^^^^^^^^^^^^^

Move the Etherscan verification istructions from Ropsten to Görli

As Ropsten test network will be deprecated in the near future, the Etherscan verification section should be update to use Görli testnet (which will remain operational).

Feature request: emit eip-4906 MetadataUpdate event when primary asset changes

For implementations and implementation examples where tokenURI returns primary asset metadataUri, it would be good to emit MetadataUpdate event when primary asset is replaced or set, so nft indexers like marketplaces and block explorers would know when to re-fetch tokenURI for the token.

https://eips.ethereum.org/EIPS/eip-4906