- I suggest moving
Relationship as it’s lookup key is the same as
_relationship and should be part of relationship
Good point, updated. Thanks.
- I suggest renaming
belongsTo in order to match existing ORM naming convention and easier for understanding.
After a thorough discussion, we humbly propose that
referred are the most apt choices to highlight the reference relationship delineated in this standard. These terms not only perfectly align with the standard’s caption but also effectively convey the active and passive aspects of the relationship. The
referring conveys the proactive action, while
referred denotes the passive reception. We concur with the interpretations of
belongsTo and are eager to incorporate your insights into our understanding.
- I suggest remove
createdTimestamp as it and the require check are not necessary. When minting NFT you would emit rather than create a timeStamp storage. Moreover, referred is not necessary to be predecessor as relationship can be create asynchronous.
A key principle of our standard is that an NFT should reference content already accepted by the community (a time-based sequence known by participants). Global timestamps for NFTs are therefore essential, serving to prevent conflicting states (akin to concurrency issues in transaction processing and block organization). This necessitates the verification of NFT timestamps at the time of their creation.
Also, to optimize timestamp synchronization, we define ‘createdTimestamp = block.timestamp.’ This approach is fundamental in our standard, reinforcing the need for NFTs to reference pre-approved content.
Given that the granularity of references is tied to the block timestamp, discerning the order of two NFTs in the same block becomes impractical. This standard is crucial in ensuring clarity and integrity in references, effectively avoiding ambiguities and potential conflicts from ‘in-block references’.
- I suggest remove getter function
referredOf instead just make
_relationship to be public which would brings in the getter function.
First, the current
referredOf allow cross-contract looking up, while this cannot be done by directly accessing
Secondly, only if privacy isn’t a concern, making
_relationship public simplifies the contract by relying on Solidity’s automatically generated getters. However, if you need to control the visibility of the data, keeping the state variable private and providing specific getter functions would be the best approach. Making
_relationship public could inadvertently expose sensitive relationship data between tokens. For example, if
_relationship includes details about specific users’ interactions or transactions or some private extensible parameters (in the updated version, we specifically highlight the
Relationship can be extended to meet different requirements), always making this data public could reveal users’ behavior patterns or preferences, leading to potential privacy breaches. Keeping such details private and using controlled getter functions prevents unintended data exposure.
- Following previously can also remove
convertMap needs to be used to return the entire mapping. Even if we set the
_relationship to public, Solidity can only generate getter functions for public state variables for a given key and a given address (whereas we actually don’t know how many addresses are stored inside). The getter function will not return the mappings contained within the struct, it will only allow you to retrieve individual values from those mappings by specifying their keys, which does not satisfy our definition of the emitted event. Therefore a separate function
convertMap needs used to retrieve the entire contents of a mapping within a struct.
- I suggest the relationship diagram to show the relationship clearly with example.
We would like to refer you to this link http://arxiv.org/abs/2402.06459 (Fig. 2) where a use case is presented.
- Overall I think EIP-5521 is not backward compatible as when setting the referred relationship, it requires the existing contract to have
setNodeReferredExternal function. In order for backward compatible, the relationship should be a separate contract to store all the relationships. That way it can referred to any existing NFT.
In the updated version, the
setNodeReferredExternal function is conditionally executed based on a successful interface verification. This selective invocation ensures compatibility with existing contracts. For functions like
referredOf, where cross-contract interactions are crucial, interface verification is essential and must return true. This design ensures that EIP-5521 can integrate with contracts that conform to the specified interfaces, thus maintaining backward compatibility while enabling new features.