[REG-1440] [WIP] feat: updating ENSCustody contract to support wallet migration #369
Conversation
Contracts size report
|
| @@ -215,15 +217,33 @@ contract ENSCustody is | |||
| } | |||
|
|
|||
| function safeTransfer(address to, uint256 tokenId) external onlyTokenOwner(tokenId) { | |||
| this.safeTransfer(to, tokenId, false); | |||
There was a problem hiding this comment.
While calling
this.safeTransfer(to, tokenId, false);
msg.sender will be contract address in the second function, which I suppose will most likely fail on onlyTokenOwner check.
Using call through this. in smart contracts is a highly unrecommended practice. Better to make lower function public and calling it like internal one
| } | ||
|
|
||
| receive() external payable {} | ||
|
|
||
| function multicall(bytes[] calldata data) public returns (bytes[] memory results) { | ||
| bytes[] memory _data = data; |
There was a problem hiding this comment.
you can just mark data as memory in parameters
| } | ||
|
|
||
| receive() external payable {} | ||
|
|
||
| function multicall(bytes[] calldata data) public returns (bytes[] memory results) { |
There was a problem hiding this comment.
we discussed with Nick and Kiryl that it is better not to include generic multicall here but to add just butchTransfer
which will take an array of tokens to transfer and perform a cycle of transfers. It must be enough for our case here.
There was a problem hiding this comment.
Do you mean something like this?
struct TransferParams {
address from;
uint256 to;
uint256 tokenId;
bytes signature
}
function BulkTransfer(TransferParams[] params) {
for (uint256 i = 0; i < params.length; i++) {
TransferParams calldata p = params[i];
// check owner, check signature, invalidate token nonce, etc.
_park(p.tokenId, p.to);
}
}
Why can't we reuse existing meta tx functionality to handle validations for each transfer?
If we check the function signature in calldata to restrict multicall only to safeTransfer, would that work?
There was a problem hiding this comment.
that's what I ment, to reuse existing meta functionality but for a narrow field of bulk transfer and not for generic multicall.
PR Checklist
1. Contracts versioning
patchversion of the contracts is increased if changes have been made to theUNSRegistry,MintingManager,ProxyReader,ENSCustodycontracts.minorversion of the contracts is increased if breaking changes have been made to theUNSRegistry,MintingManager,ProxyReader,ENSCustodycontracts. It includes changes of interfaces.2. Contracts licensing
3. Coverage
4. Configs versioning
uns-config.jsonis increased if changes have been made to the config.ens-config.jsonis increased if changes have been made to the config.resolver-keys.jsonis increased if changes have been made to the config.ens-resolver-keys.jsonis increased if changes have been made to the config.5. Package versioning
patchversion of package is increased if valuable changes have been made to the package. It includes contracts update, configs update, etc.major.minorversion of package is synced with version ofUNSRegistrycontract.CHANGELOGis updated with short description for the new version.6. Code review
resolver-keys.jsoncode review is required from DevTools teamens-resolver-keys.jsoncode review is required from DevTools team