You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Most, if not all, retrieve functions do not check the expiry time of a delegation. You can see in this example function (retrieveGlobalStatusOfDelegation) that there is no check of expiry time. A global delegation may be expired, but this function will return back true as the length of the array is greater than 0.
function retrieveGlobalStatusOfDelegation(address_delegatorAddress, address_collectionAddress, address_delegationAddress, uint256_useCase) publicviewreturns (bool) {
This can lead to confusion for developers trying to use this delegation registry, especially if they aren't fluent in Solidity. Perhaps this stems from my expectations based on the description of the retrieve functions, but in my opinion, the code should make it as simple as possible and not rely on logic to be implemented elsewhere. This registry should act as the source of truth any time I call it and not rely on logic to be implemented elsewhere. If I were going to check these functions in another contract, I want the truth returned to me so I can use it for my own purposes.
The text was updated successfully, but these errors were encountered:
Something else that is likely important is that retrieveDelegators does not check expiry times which could allow for expired sub-delegators to act after their time has expired.
Taking into account the expiry date of a delegation clearly depends on the developer who will integrate the smart contract. For this purpose we have developed a specific retrieve function namely, retrieveActiveDelegators(...) that takes into consideration a specific epoch time.
By executing this function you can retrieve only the active delegators for a specific delegation address based on the expiry date.
If the contract defines an expiry time, shouldn't that be enforced everywhere? Seems like an easy way to lead to improper integration with third party apps. At least naming the functions something like retrieveAllHistoricalDelegates would give a clue to devs on what is returned.
Additionally, better documentation + natspec comments on these considerations would be incredibly helpful. Right now, devs just have to guess at the purpose of a function based on one comment in the source code and aren't aware of any pitfalls, such as not checking expiry time.
Problem Statement
Most, if not all, retrieve functions do not check the expiry time of a delegation. You can see in this example function (
retrieveGlobalStatusOfDelegation
) that there is no check of expiry time. A global delegation may be expired, but this function will return backtrue
as the length of the array is greater than 0.nftdelegation/smart_contracts/DelegationManagement.sol
Line 604 in 50e3d17
This can lead to confusion for developers trying to use this delegation registry, especially if they aren't fluent in Solidity. Perhaps this stems from my expectations based on the description of the retrieve functions, but in my opinion, the code should make it as simple as possible and not rely on logic to be implemented elsewhere. This registry should act as the source of truth any time I call it and not rely on logic to be implemented elsewhere. If I were going to check these functions in another contract, I want the truth returned to me so I can use it for my own purposes.
The text was updated successfully, but these errors were encountered: