Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REG-1440] [WIP] feat: updating ENSCustody contract to support wallet migration #369

Closed
wants to merge 1 commit into from
+175 −18
Closed

[REG-1440] [WIP] feat: updating ENSCustody contract to support wallet migration #369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions artifacts/ENSCustody.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion artifacts/abi/ENSCustody.json
View file
Open in desktop

Large diffs are not rendered by default.

28 changes: 24 additions & 4 deletions contracts/custody/ENSCustody.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {IENSCustody, Unauthorised, InvalidToken, UnknownToken, CustodyNotEnoughB
import {InvalidForwardedToken, ERC2771RegistryContext} from '../metatx/ERC2771RegistryContext.sol';
import {Forwarder} from '../metatx/Forwarder.sol';
import {MinterRole} from '../roles/MinterRole.sol';
import {Multicall} from '../utils/Multicall.sol';

contract ENSCustody is
Initializable,
Expand All @@ -27,7 +28,8 @@ contract ENSCustody is
ERC2771RegistryContext,
Forwarder,
MinterRole,
IENSCustody
IENSCustody,
Multicall
{
string public constant NAME = 'ENS Custody';
string public constant VERSION = '0.1.3';
Expand Down Expand Up @@ -215,15 +217,33 @@ contract ENSCustody is
}

function safeTransfer(address to, uint256 tokenId) external onlyTokenOwner(tokenId) {
this.safeTransfer(to, tokenId, false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

function safeTransfer(address to, uint256 tokenId, bool internalTransfer) external onlyTokenOwner(tokenId) {
_protectTokenOperation(tokenId);
StorageSlotUpgradeable.getAddressSlot(keccak256(abi.encodePacked(_OWNER_PREFIX_SLOT, tokenId))).value = address(0);
if (internalTransfer) {
_park(tokenId, to);
} else {
StorageSlotUpgradeable.getAddressSlot(keccak256(abi.encodePacked(_OWNER_PREFIX_SLOT, tokenId))).value = address(0);

INameWrapper _wrapper = INameWrapper(StorageSlotUpgradeable.getAddressSlot(_ENS_WRAPPER_SLOT).value);
_wrapper.safeTransferFrom(address(this), to, tokenId, 1, '');
INameWrapper _wrapper = INameWrapper(StorageSlotUpgradeable.getAddressSlot(_ENS_WRAPPER_SLOT).value);
_wrapper.safeTransferFrom(address(this), to, tokenId, 1, '');
}
}

receive() external payable {}

function multicall(bytes[] calldata data) public returns (bytes[] memory results) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@DancingAxolotl DancingAxolotl Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what I ment, to reuse existing meta functionality but for a narrow field of bulk transfer and not for generic multicall.

bytes[] memory _data = data;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can just mark data as memory in parameters

if (isTrustedForwarder(msg.sender)) {
for (uint256 i = 0; i < data.length; i++) {
_data[i] = _buildData(_msgSender(), _msgToken(), data[i], '');
}
}
return _multicall(_data);
}

function _register(
string calldata name,
address owner,
Expand Down
1 change: 1 addition & 0 deletions contracts/utils/Multicall.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import '@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol';
abstract contract Multicall {
/**
* @dev Receives and executes a batch of function calls on this contract.
* @custom:oz-upgrades-unsafe-allow-reachable delegatecall
*/
function _multicall(bytes[] memory data) internal returns (bytes[] memory results) {
results = new bytes[](data.length);
Expand Down
2 changes: 1 addition & 1 deletion dist/sandbox/state.json
View file
Open in desktop

Large diffs are not rendered by default.

30 changes: 25 additions & 5 deletions dist/types/contracts/custody/ENSCustody.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export declare namespace IForwarder {
};
}
export interface ENSCustodyInterface extends Interface {
getFunction(nameOrSignature: "DEFAULT_ADMIN_ROLE" | "MINTER_ROLE" | "NAME" | "VERSION" | "addMinter" | "addMinters" | "closeMinter" | "commit" | "execute" | "getRoleAdmin" | "grantRole" | "hasRole" | "initialize" | "isMinter" | "isTrustedForwarder" | "makeCommitment" | "nonceOf" | "onERC1155BatchReceived" | "onERC1155Received" | "onERC721Received" | "owner" | "ownerOf" | "register" | "removeMinter" | "removeMinters" | "renew" | "renounceMinter" | "renounceOwnership" | "renounceRole" | "rentPrice" | "revokeRole" | "rotateMinter" | "safeTransfer" | "setBaseRegistrar" | "supportsInterface" | "transferOwnership" | "verify"): FunctionFragment;
getFunction(nameOrSignature: "DEFAULT_ADMIN_ROLE" | "MINTER_ROLE" | "NAME" | "VERSION" | "addMinter" | "addMinters" | "closeMinter" | "commit" | "execute" | "getRoleAdmin" | "grantRole" | "hasRole" | "initialize" | "isMinter" | "isTrustedForwarder" | "makeCommitment" | "multicall" | "nonceOf" | "onERC1155BatchReceived" | "onERC1155Received" | "onERC721Received" | "owner" | "ownerOf" | "register" | "removeMinter" | "removeMinters" | "renew" | "renounceMinter" | "renounceOwnership" | "renounceRole" | "rentPrice" | "revokeRole" | "rotateMinter" | "safeTransfer(address,uint256,bool)" | "safeTransfer(address,uint256)" | "setBaseRegistrar" | "supportsInterface" | "transferOwnership" | "verify"): FunctionFragment;
getEvent(nameOrSignatureOrTopic: "Initialized" | "OwnershipTransferred" | "Parked" | "RoleAdminChanged" | "RoleGranted" | "RoleRevoked"): EventFragment;
encodeFunctionData(functionFragment: "DEFAULT_ADMIN_ROLE", values?: undefined): string;
encodeFunctionData(functionFragment: "MINTER_ROLE", values?: undefined): string;
Expand Down Expand Up @@ -48,6 +48,7 @@ export interface ENSCustodyInterface extends Interface {
BigNumberish,
boolean
]): string;
encodeFunctionData(functionFragment: "multicall", values: [BytesLike[]]): string;
encodeFunctionData(functionFragment: "nonceOf", values: [BigNumberish]): string;
encodeFunctionData(functionFragment: "onERC1155BatchReceived", values: [
AddressLike,
Expand Down Expand Up @@ -80,7 +81,8 @@ export interface ENSCustodyInterface extends Interface {
encodeFunctionData(functionFragment: "rentPrice", values: [string, BigNumberish]): string;
encodeFunctionData(functionFragment: "revokeRole", values: [BytesLike, AddressLike]): string;
encodeFunctionData(functionFragment: "rotateMinter", values: [AddressLike]): string;
encodeFunctionData(functionFragment: "safeTransfer", values: [AddressLike, BigNumberish]): string;
encodeFunctionData(functionFragment: "safeTransfer(address,uint256,bool)", values: [AddressLike, BigNumberish, boolean]): string;
encodeFunctionData(functionFragment: "safeTransfer(address,uint256)", values: [AddressLike, BigNumberish]): string;
encodeFunctionData(functionFragment: "setBaseRegistrar", values: [AddressLike]): string;
encodeFunctionData(functionFragment: "supportsInterface", values: [BytesLike]): string;
encodeFunctionData(functionFragment: "transferOwnership", values: [AddressLike]): string;
Expand All @@ -101,6 +103,7 @@ export interface ENSCustodyInterface extends Interface {
decodeFunctionResult(functionFragment: "isMinter", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "isTrustedForwarder", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "makeCommitment", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "multicall", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "nonceOf", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "onERC1155BatchReceived", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "onERC1155Received", data: BytesLike): Result;
Expand All @@ -117,7 +120,8 @@ export interface ENSCustodyInterface extends Interface {
decodeFunctionResult(functionFragment: "rentPrice", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "revokeRole", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "rotateMinter", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "safeTransfer", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "safeTransfer(address,uint256,bool)", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "safeTransfer(address,uint256)", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "setBaseRegistrar", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "supportsInterface", data: BytesLike): Result;
decodeFunctionResult(functionFragment: "transferOwnership", data: BytesLike): Result;
Expand Down Expand Up @@ -283,6 +287,7 @@ export interface ENSCustody extends BaseContract {
], [
string
], "view">;
multicall: TypedContractMethod<[data: BytesLike[]], [string[]], "nonpayable">;
nonceOf: TypedContractMethod<[tokenId: BigNumberish], [bigint], "view">;
onERC1155BatchReceived: TypedContractMethod<[
arg0: AddressLike,
Expand Down Expand Up @@ -362,7 +367,14 @@ export interface ENSCustody extends BaseContract {
void
], "nonpayable">;
rotateMinter: TypedContractMethod<[receiver: AddressLike], [void], "payable">;
safeTransfer: TypedContractMethod<[
"safeTransfer(address,uint256,bool)": TypedContractMethod<[
to: AddressLike,
tokenId: BigNumberish,
internalTransfer: boolean
], [
void
], "nonpayable">;
"safeTransfer(address,uint256)": TypedContractMethod<[
to: AddressLike,
tokenId: BigNumberish
], [
Expand Down Expand Up @@ -439,6 +451,7 @@ export interface ENSCustody extends BaseContract {
], [
string
], "view">;
getFunction(nameOrSignature: "multicall"): TypedContractMethod<[data: BytesLike[]], [string[]], "nonpayable">;
getFunction(nameOrSignature: "nonceOf"): TypedContractMethod<[tokenId: BigNumberish], [bigint], "view">;
getFunction(nameOrSignature: "onERC1155BatchReceived"): TypedContractMethod<[
arg0: AddressLike,
Expand Down Expand Up @@ -510,7 +523,14 @@ export interface ENSCustody extends BaseContract {
void
], "nonpayable">;
getFunction(nameOrSignature: "rotateMinter"): TypedContractMethod<[receiver: AddressLike], [void], "payable">;
getFunction(nameOrSignature: "safeTransfer"): TypedContractMethod<[
getFunction(nameOrSignature: "safeTransfer(address,uint256,bool)"): TypedContractMethod<[
to: AddressLike,
tokenId: BigNumberish,
internalTransfer: boolean
], [
void
], "nonpayable">;
getFunction(nameOrSignature: "safeTransfer(address,uint256)"): TypedContractMethod<[
to: AddressLike,
tokenId: BigNumberish
], [
Expand Down
2 changes: 1 addition & 1 deletion dist/types/contracts/custody/ENSCustody.d.ts.map
View file
Open in desktop

Large diffs are not rendered by default.

34 changes: 33 additions & 1 deletion dist/types/factories/contracts/custody/ENSCustody__factory.d.ts
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/types/factories/contracts/custody/ENSCustody__factory.d.ts.map
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 43 additions & 1 deletion dist/types/factories/contracts/custody/ENSCustody__factory.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion sandbox/state.json
View file
Open in desktop

Large diffs are not rendered by default.

Loading