[CodeArena] - [G-11] : Change mint to external #76
Conversation
| originalAsset[tokenId] = originalTokenId != 0 ? originalTokenId : _replicatedTokenId; | ||
|
|
||
| return tokenId; | ||
| function mint(address _owner, uint256 _replicatedTokenId) external onlyFactory returns (uint256) { |
There was a problem hiding this comment.
❓ Do we really save gas by calling an external function that calls another function and passes parameters, instead of a public function?
There was a problem hiding this comment.
Calling a public function costs more gas, however we're not saving gas in this case. It's just "cleaner", so I was also wondering if we should close this PR.
There was a problem hiding this comment.
I see why it is cleaner but here I'd advocate for the most efficient solution
| /// @param _owner The account address that signed the transaction | ||
| /// @param _replicatedTokenId The token id of the replicated asset, 0 if no replication | ||
| /// @return The minted token's id | ||
| function _mintFromReplicated(address _owner, uint256 _replicatedTokenId) internal returns (uint256) { |
There was a problem hiding this comment.
The name of the function is inaccurate as it can be called with _replicatedTokenId being 0, when it is not a copy.
| function _mintFromReplicated(address _owner, uint256 _replicatedTokenId) internal returns (uint256) { | |
| function _mint(address _owner, uint256 _replicatedTokenId) internal returns (uint256) { |
There was a problem hiding this comment.
I needed a name different from _mint (already exists). Even if the parameter can be 0, the parameter is _replicatedTokenId .
Other possibilities :
_mintWithReplication_mintAndHandleReplication
adrien-supizet
removed
the
To review
|
@adrien-supizet Change this PR as "draft", we should discuss about it (pro and cons). |
|
After discussing, we prefer the most efficient solution. |
Issue => code-423n4/2021-11-nested-findings#29
Note from the issue :