ersanyakit
commented
8 months ago @0xacme It seems like the ERC1155 standard is much more suitable for ERC404... It could get rid of the loops in minting operations. May I know why you prefer ERC721?
Closed 0xacme closed 8 months ago
ersanyakit
commented
8 months ago @0xacme It seems like the ERC1155 standard is much more suitable for ERC404... It could get rid of the loops in minting operations. May I know why you prefer ERC721?
Audit Revisions
This PR aims to be the cutoff for audit-related contract revisions. I've provided comments on each identified issue, and have linked commits (if not included in this PR). A handful of additional minor changes have occurred, and diff checking would be recommended, though these will be addresses in this documentation as well.
Findings
Additional Changes
Some renaming has occurred for clarity, and a couple of view functions have been introduced. Diff checking strongly recommended to ensure nothing is excluded from these audit notes.
ERC404Legacy was added, this is just the implementation used for Pandora.
ERC404U16 was also added, which packs token ids in _owned and the id bank under the assumption that they are less than 65535. This was introduced to optimize storage.
ID pathing has been adjusted such that all id's are prefixed by 1 << 255 to avoid conflicts within integrating protocols.
Introduced conflicting events. While signatures will conflict, this shouldn't be a substantial problem in practice and is easier to integrate around vs entirely new event sigs. It may not be a final solution here, though indexers typically look for a mix of topics + signature when handling these signatures (which in effect already do conflict, just not within the same contract).