Skip to content
This repository has been archived by the owner on May 9, 2024. It is now read-only.

Commit

Permalink
removed adminSetHandlerAddress (#179)
Browse files Browse the repository at this point in the history 
* removed adminSetHandler

* fix test
  • Loading branch information
@steviezhang
steviezhang authored Jun 15, 2020
1 parent 5a696a2 commit 81804a8
Show file tree
Hide file tree
Showing 21 changed files with 43 additions and 62 deletions.
17 changes: 2 additions & 15 deletions contracts/Bridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,16 +212,6 @@ contract Bridge is Pausable, AccessControl {
_totalRelayers--;
}

/**
@notice Maps the {handlerAddress} to {resourceID} in {_resourceIDToHandlerAddress}.
@notice Only callable by an address that currently has the admin role.
@param handlerAddress Address of handler resource will be mapped to.
@param resourceID ResourceID to be used when making deposits.
*/
function adminSetHandlerAddress(address handlerAddress, bytes32 resourceID) external onlyAdmin {
_setHandlerAddress(handlerAddress, resourceID);
}

/**
@notice Sets a new resource for handler contracts that use the IERCHandler interface,
and maps the {handlerAddress} to {resourceID} in {_resourceIDToHandlerAddress}.
Expand All @@ -231,7 +221,7 @@ contract Bridge is Pausable, AccessControl {
@param tokenAddress Address of contract to be called when a deposit is made and a deposited is executed.
*/
function adminSetResource(address handlerAddress, bytes32 resourceID, address tokenAddress) external onlyAdmin {
_setHandlerAddress(handlerAddress, resourceID);
_resourceIDToHandlerAddress[resourceID] = handlerAddress;
IERCHandler handler = IERCHandler(handlerAddress);
handler.setResource(resourceID, tokenAddress);
}
Expand All @@ -251,7 +241,7 @@ contract Bridge is Pausable, AccessControl {
bytes4 depositFunctionSig,
bytes4 executeFunctionSig
) external onlyAdmin {
_setHandlerAddress(handlerAddress, resourceID);
_resourceIDToHandlerAddress[resourceID] = handlerAddress;
IGenericHandler handler = IGenericHandler(handlerAddress);
handler.setResource(resourceID, contractAddress, depositFunctionSig, executeFunctionSig);
}
Expand Down Expand Up @@ -439,7 +429,4 @@ contract Bridge is Pausable, AccessControl {
}
}

function _setHandlerAddress(address handlerAddress, bytes32 resourceID) internal {
_resourceIDToHandlerAddress[resourceID] = handlerAddress;
}
}
6 changes: 0 additions & 6 deletions contracts/handlers/GenericHandler.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,6 @@ contract GenericHandler is IGenericHandler {
bytes4 depositFunctionSig,
bytes4 executeFunctionSig
) external onlyBridge override {
require(_resourceIDToContractAddress[resourceID] == address(0), "resourceID already has a corresponding contract address");

bytes32 currentResourceID = _contractAddressToResourceID[contractAddress];
bytes32 emptyBytes;
require(keccak256(abi.encodePacked((currentResourceID))) == keccak256(abi.encodePacked((emptyBytes))),
"contract address already has corresponding resourceID");

_setResource(resourceID, contractAddress, depositFunctionSig, executeFunctionSig);
}
Expand Down
2 changes: 1 addition & 1 deletion test/contractBridge/admin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ contract('Bridge - [admin]', async accounts => {

assert.equal(await BridgeInstance._resourceIDToHandlerAddress.call(resourceID), Ethers.constants.AddressZero);

TruffleAssert.passes(await BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, resourceID));
TruffleAssert.passes(await BridgeInstance.adminSetResource(ERC20HandlerInstance.address, resourceID, ERC20MintableInstance.address));
assert.equal(await BridgeInstance._resourceIDToHandlerAddress.call(resourceID), ERC20HandlerInstance.address);
});

Expand Down
2 changes: 1 addition & 1 deletion test/contractBridge/cancelDepositProposal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)

await Promise.all([
DestinationERC20MintableInstance.grantRole(await DestinationERC20MintableInstance.MINTER_ROLE(), DestinationERC20HandlerInstance.address),
BridgeInstance.adminSetHandlerAddress(DestinationERC20HandlerInstance.address, resourceID)
BridgeInstance.adminSetResource(DestinationERC20HandlerInstance.address, resourceID, DestinationERC20MintableInstance.address)
]);

vote = (relayer) => BridgeInstance.voteProposal(originChainID, expectedDepositNonce, resourceID, depositDataHash, {from: relayer});
Expand Down
2 changes: 1 addition & 1 deletion test/contractBridge/depositGeneric.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ contract('Bridge - [deposit - Generic]', async () => {
initialDepositFunctionSignatures,
initialExecuteFunctionSignatures);

await BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, resourceID);
await BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, resourceID, initialContractAddresses[0], initialDepositFunctionSignatures[0], initialExecuteFunctionSignatures[0]);

depositData = Helpers.createGenericDepositData(resourceID, '0xdeadbeef');
});
Expand Down
2 changes: 1 addition & 1 deletion test/contractBridge/fee.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ contract('Bridge - [fee]', async (accounts) => {
initialDepositFunctionSignatures,
initialExecuteFunctionSignatures);

await BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, resourceID);
await BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, resourceID, initialContractAddresses[0], initialDepositFunctionSignatures[0], initialExecuteFunctionSignatures[0]);

depositData = Helpers.createGenericDepositData(resourceID, '0xdeadbeef');
});
Expand Down
2 changes: 1 addition & 1 deletion test/contractBridge/voteDepositProposal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ contract('Bridge - [voteProposal with relayerThreshold == 3]', async (accounts)

await Promise.all([
DestinationERC20MintableInstance.grantRole(await DestinationERC20MintableInstance.MINTER_ROLE(), DestinationERC20HandlerInstance.address),
BridgeInstance.adminSetHandlerAddress(DestinationERC20HandlerInstance.address, resourceID)
BridgeInstance.adminSetResource(DestinationERC20HandlerInstance.address, resourceID, DestinationERC20MintableInstance.address)
]);

vote = (relayer) => BridgeInstance.voteProposal(originChainID, expectedDepositNonce, resourceID, depositDataHash, {from: relayer});
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/erc20/differentChainsMock.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ contract('E2E ERC20 - Two EVM Chains', async accounts => {
OriginERC20MintableInstance.approve(OriginERC20HandlerInstance.address, depositAmount, { from: depositerAddress }),
OriginERC20MintableInstance.grantRole(await OriginERC20MintableInstance.MINTER_ROLE(), OriginERC20HandlerInstance.address),
DestinationERC20MintableInstance.grantRole(await DestinationERC20MintableInstance.MINTER_ROLE(), DestinationERC20HandlerInstance.address),
OriginBridgeInstance.adminSetHandlerAddress(OriginERC20HandlerInstance.address, originResourceID),
DestinationBridgeInstance.adminSetHandlerAddress(DestinationERC20HandlerInstance.address, destinationResourceID)
OriginBridgeInstance.adminSetResource(OriginERC20HandlerInstance.address, originResourceID, OriginERC20MintableInstance.address),
DestinationBridgeInstance.adminSetResource(DestinationERC20HandlerInstance.address, destinationResourceID, DestinationERC20MintableInstance.address)
]);

originDepositData = Helpers.createERCDepositData(originResourceID, depositAmount, 32, recipientAddress);
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/erc20/sameChain.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract('E2E ERC20 - Same Chain', async accounts => {

await Promise.all([
ERC20MintableInstance.mint(depositerAddress, initialTokenAmount),
BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, resourceID)
BridgeInstance.adminSetResource(ERC20HandlerInstance.address, resourceID, ERC20MintableInstance.address)
]);

await ERC20MintableInstance.approve(ERC20HandlerInstance.address, depositAmount, { from: depositerAddress });
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/erc721/differentChainsMock.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ contract('E2E ERC721 - Two EVM Chains', async accounts => {
await Promise.all([
OriginERC721MintableInstance.approve(OriginERC721HandlerInstance.address, tokenID, { from: depositerAddress }),
DestinationERC721MintableInstance.grantRole(await DestinationERC721MintableInstance.MINTER_ROLE(), DestinationERC721HandlerInstance.address),
OriginBridgeInstance.adminSetHandlerAddress(OriginERC721HandlerInstance.address, originResourceID),
DestinationBridgeInstance.adminSetHandlerAddress(DestinationERC721HandlerInstance.address, destinationResourceID)
OriginBridgeInstance.adminSetResource(OriginERC721HandlerInstance.address, originResourceID, OriginERC721MintableInstance.address),
DestinationBridgeInstance.adminSetResource(DestinationERC721HandlerInstance.address, destinationResourceID, DestinationERC721MintableInstance.address)
]);

originDepositData = Helpers.createERCDepositData(originResourceID, tokenID, 32, recipientAddress);
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/erc721/sameChain.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ contract('E2E ERC721 - Same Chain', async accounts => {

await Promise.all([
ERC721MintableInstance.mint(depositerAddress, tokenID, depositMetadata),
BridgeInstance.adminSetHandlerAddress(ERC721HandlerInstance.address, resourceID)
BridgeInstance.adminSetResource(ERC721HandlerInstance.address, resourceID, ERC721MintableInstance.address)
]);

await ERC721MintableInstance.approve(ERC721HandlerInstance.address, tokenID, { from: depositerAddress });
Expand Down
14 changes: 7 additions & 7 deletions test/gasBenchmarks/deposits.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,13 @@ contract('Gas Benchmark - [Deposits]', async (accounts) => {
await Promise.all([
ERC20MintableInstance.approve(ERC20HandlerInstance.address, erc20TokenAmount, { from: depositerAddress }),
ERC721MintableInstance.approve(ERC721HandlerInstance.address, erc721TokenID, { from: depositerAddress }),
BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, erc20ResourceID),
BridgeInstance.adminSetHandlerAddress(ERC721HandlerInstance.address, erc721ResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, centrifugeAssetResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, noArgumentResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, oneArgumentResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, twoArgumentsResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, threeArgumentsResourceID)
BridgeInstance.adminSetResource(ERC20HandlerInstance.address, erc20ResourceID, ERC20MintableInstance.address),
BridgeInstance.adminSetResource(ERC721HandlerInstance.address, erc721ResourceID, ERC721MintableInstance.address),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, centrifugeAssetResourceID, genericInitialContractAddresses[0], genericInitialDepositFunctionSignatures[0], genericInitialExecuteFunctionSignatures[0]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, noArgumentResourceID, genericInitialContractAddresses[1], genericInitialDepositFunctionSignatures[1], genericInitialExecuteFunctionSignatures[1]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, oneArgumentResourceID, genericInitialContractAddresses[2], genericInitialDepositFunctionSignatures[2], genericInitialExecuteFunctionSignatures[2]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, twoArgumentsResourceID, genericInitialContractAddresses[3], genericInitialDepositFunctionSignatures[3], genericInitialExecuteFunctionSignatures[3]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, threeArgumentsResourceID, genericInitialContractAddresses[4], genericInitialDepositFunctionSignatures[4], genericInitialExecuteFunctionSignatures[4])
]);
});

Expand Down
14 changes: 7 additions & 7 deletions test/gasBenchmarks/executeProposal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,13 @@ contract('Gas Benchmark - [Execute Proposal]', async (accounts) => {
await Promise.all([
ERC20MintableInstance.approve(ERC20HandlerInstance.address, erc20TokenAmount, { from: depositerAddress }),
ERC721MintableInstance.approve(ERC721HandlerInstance.address, erc721TokenID, { from: depositerAddress }),
BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, erc20ResourceID),
BridgeInstance.adminSetHandlerAddress(ERC721HandlerInstance.address, erc721ResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, centrifugeAssetResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, noArgumentResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, oneArgumentResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, twoArgumentsResourceID),
BridgeInstance.adminSetHandlerAddress(GenericHandlerInstance.address, threeArgumentsResourceID)
BridgeInstance.adminSetResource(ERC20HandlerInstance.address, erc20ResourceID, ERC20MintableInstance.address),
BridgeInstance.adminSetResource(ERC721HandlerInstance.address, erc721ResourceID, ERC721MintableInstance.address),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, centrifugeAssetResourceID, genericInitialContractAddresses[0], genericInitialDepositFunctionSignatures[0], genericInitialExecuteFunctionSignatures[0]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, noArgumentResourceID, genericInitialContractAddresses[1], genericInitialDepositFunctionSignatures[1], genericInitialExecuteFunctionSignatures[1]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, oneArgumentResourceID, genericInitialContractAddresses[2], genericInitialDepositFunctionSignatures[2], genericInitialExecuteFunctionSignatures[2]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, twoArgumentsResourceID, genericInitialContractAddresses[3], genericInitialDepositFunctionSignatures[3], genericInitialExecuteFunctionSignatures[3]),
BridgeInstance.adminSetGenericResource(GenericHandlerInstance.address, threeArgumentsResourceID, genericInitialContractAddresses[4], genericInitialDepositFunctionSignatures[4], genericInitialExecuteFunctionSignatures[4])
]);
});

Expand Down
2 changes: 1 addition & 1 deletion test/gasBenchmarks/voteProposal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ contract('Gas Benchmark - [Vote Proposal]', async (accounts) => {

await Promise.all([
ERC20MintableInstance.approve(ERC20HandlerInstance.address, erc20TokenAmount, { from: depositerAddress }),
BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, erc20ResourceID),
BridgeInstance.adminSetResource(ERC20HandlerInstance.address, erc20ResourceID, ERC20MintableInstance.address),
]);
});

Expand Down
4 changes: 2 additions & 2 deletions test/handlers/erc20/deposit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ contract('ERC20Handler - [Deposit ERC20]', async (accounts) => {

await Promise.all([
ERC20MintableInstance.approve(ERC20HandlerInstance.address, tokenAmount, { from: depositerAddress }),
BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, resourceID)
BridgeInstance.adminSetResource(ERC20HandlerInstance.address, resourceID, ERC20MintableInstance.address)
]);
});

it('[sanity] depositer owns tokenAmount od ERC20', async () => {
it('[sanity] depositer owns tokenAmount of ERC20', async () => {
const depositerBalance = await ERC20MintableInstance.balanceOf(depositerAddress);
assert.equal(tokenAmount, depositerBalance);
});
Expand Down
4 changes: 2 additions & 2 deletions test/handlers/erc20/depositBurn.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ contract('ERC20Handler - [Deposit Burn ERC20]', async (accounts) => {

await Promise.all([
ERC20MintableInstance1.approve(ERC20HandlerInstance.address, depositAmount, { from: depositerAddress }),
BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, resourceID1),
BridgeInstance.adminSetHandlerAddress(ERC20HandlerInstance.address, resourceID2),
BridgeInstance.adminSetResource(ERC20HandlerInstance.address, resourceID1, ERC20MintableInstance1.address),
BridgeInstance.adminSetResource(ERC20HandlerInstance.address, resourceID2, ERC20MintableInstance2.address),
]);

depositData = Helpers.createERCDepositData(resourceID1, depositAmount, 32, recipientAddress);
Expand Down
4 changes: 2 additions & 2 deletions test/handlers/erc20/setResourceIDAndContractAddress.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ contract('ERC20Handler - [setResourceIDAndContractAddress]', async () => {
const secondERC20ResourceID = [Ethers.utils.hexZeroPad((ERC20MintableInstance2.address + Ethers.utils.hexlify(chainID).substr(2)), 32)];
ERC20HandlerInstance2 = await ERC20HandlerContract.new(BridgeInstance.address, secondERC20ResourceID, [ERC20MintableInstance2.address], burnableContractAddresses);

await BridgeInstance.adminSetHandlerAddress(ERC20MintableInstance2.address, initialResourceIDs[0]);
await BridgeInstance.adminSetResource(ERC20HandlerInstance2.address, initialResourceIDs[0], ERC20MintableInstance2.address);

const bridgeHandlerAddress = await BridgeInstance._resourceIDToHandlerAddress.call(initialResourceIDs[0]);
assert.strictEqual(bridgeHandlerAddress.toLowerCase(), ERC20MintableInstance2.address.toLowerCase());
assert.strictEqual(bridgeHandlerAddress.toLowerCase(), ERC20HandlerInstance2.address.toLowerCase());
});

it('existing resourceID should be replaced by new resourceID in handler', async () => {
Expand Down
2 changes: 1 addition & 1 deletion test/handlers/erc721/deposit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ contract('ERC721Handler - [Deposit ERC721]', async (accounts) => {

await Promise.all([
ERC721MintableInstance.approve(ERC721HandlerInstance.address, tokenID, { from: depositerAddress }),
BridgeInstance.adminSetHandlerAddress(ERC721HandlerInstance.address, resourceID)
BridgeInstance.adminSetResource(ERC721HandlerInstance.address, resourceID, ERC721MintableInstance.address)
]);
});

Expand Down
4 changes: 2 additions & 2 deletions test/handlers/erc721/depositBurn.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ contract('ERC721Handler - [Deposit Burn ERC721]', async (accounts) => {

await Promise.all([
ERC721MintableInstance1.approve(ERC721HandlerInstance.address, tokenID, { from: depositerAddress }),
BridgeInstance.adminSetHandlerAddress(ERC721HandlerInstance.address, resourceID1),
BridgeInstance.adminSetHandlerAddress(ERC721HandlerInstance.address, resourceID2),
BridgeInstance.adminSetResource(ERC721HandlerInstance.address, resourceID1, ERC721MintableInstance1.address),
BridgeInstance.adminSetResource(ERC721HandlerInstance.address, resourceID2, ERC721MintableInstance2.address),
]);

depositData = Helpers.createERCDepositData(resourceID1, tokenID, 32, recipientAddress);
Expand Down
Loading

0 comments on commit 81804a8

Please sign in to comment.