Skip to content

Commit

Permalink
feat: removed the default evm address from remote address validator (#…
Browse files Browse the repository at this point in the history
…112)

* removed the default evm address from remote address validator

* fixed a namign issue in tests

---------

Co-authored-by: Milap Sheth <[email protected]>
  • Loading branch information
Foivos and milapsheth authored Oct 9, 2023
1 parent 83ff4ee commit 111fed9
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 161 deletions.
11 changes: 1 addition & 10 deletions contracts/interfaces/IRemoteAddressValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ interface IRemoteAddressValidator {
error ZeroAddress();
error LengthMismatch();
error ZeroStringLength();
error UntrustedChain();

event TrustedAddressAdded(string sourceChain, string sourceAddress);
event TrustedAddressRemoved(string sourceChain);
Expand All @@ -19,16 +20,6 @@ interface IRemoteAddressValidator {
*/
function chainName() external view returns (string memory);

/**
* @notice Returns the interchain token address
*/
function interchainTokenServiceAddress() external view returns (address);

/**
* @notice Returns the interchain token address to string to lower case hash, which is used to compare with incoming calls.
*/
function interchainTokenServiceAddressHash() external view returns (bytes32);

/**
* @dev Validates that the sender is a valid interchain token service address
* @param sourceChain Source chain of the transaction
Expand Down
54 changes: 4 additions & 50 deletions contracts/remote-address-validator/RemoteAddressValidator.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,13 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable {
mapping(string => string) public remoteAddresses;
string public chainName;

address public immutable interchainTokenServiceAddress;
bytes32 public immutable interchainTokenServiceAddressHash;

/**
* @dev Store the interchain token service address as string across two immutable variables to avoid recomputation and save gas
*/
uint256 private immutable interchainTokenServiceAddress1;
uint256 private immutable interchainTokenServiceAddress2;

bytes32 private constant CONTRACT_ID = keccak256('remote-address-validator');

/**
* @dev Constructs the RemoteAddressValidator contract, both array parameters must be equal in length
* @param _interchainTokenServiceAddress Address of the interchain token service
* @dev Constructs the RemoteAddressValidator contract, both array parameters must be equal in length.
* @param chainName_ The name of the current chain.
*/
constructor(address _interchainTokenServiceAddress, string memory chainName_) {
if (_interchainTokenServiceAddress == address(0)) revert ZeroAddress();

interchainTokenServiceAddress = _interchainTokenServiceAddress;

string memory interchainTokenServiceAddressString = interchainTokenServiceAddress.toString();
interchainTokenServiceAddressHash = keccak256(bytes(interchainTokenServiceAddressString));

uint256 p1;
uint256 p2;

assembly {
p1 := mload(add(interchainTokenServiceAddressString, 32))
p2 := mload(add(interchainTokenServiceAddressString, 64))
}

interchainTokenServiceAddress1 = p1;
interchainTokenServiceAddress2 = p2;

constructor(string memory chainName_) {
if (bytes(chainName_).length == 0) revert ZeroStringLength();
chainName = chainName_;
}
Expand Down Expand Up @@ -89,21 +62,6 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable {
return s;
}

/**
* @dev Return the interchain token service address as a string by constructing it from the two immutable variables caching it
*/
function _interchainTokenServiceAddressString() internal view returns (string memory interchainTokenServiceAddressString) {
interchainTokenServiceAddressString = new string(42);

uint256 p1 = interchainTokenServiceAddress1;
uint256 p2 = interchainTokenServiceAddress2;

assembly {
mstore(add(interchainTokenServiceAddressString, 32), p1)
mstore(add(interchainTokenServiceAddressString, 64), p2)
}
}

/**
* @dev Validates that the sender is a valid interchain token service address
* @param sourceChain Source chain of the transaction
Expand All @@ -114,10 +72,6 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable {
string memory sourceAddressNormalized = _lowerCase(sourceAddress);
bytes32 sourceAddressHash = keccak256(bytes(sourceAddressNormalized));

if (sourceAddressHash == interchainTokenServiceAddressHash) {
return true;
}

return sourceAddressHash == remoteAddressHashes[sourceChain];
}

Expand Down Expand Up @@ -158,7 +112,7 @@ contract RemoteAddressValidator is IRemoteAddressValidator, Upgradable {
remoteAddress = remoteAddresses[chainName_];

if (bytes(remoteAddress).length == 0) {
remoteAddress = _interchainTokenServiceAddressString();
revert UntrustedChain();
}
}
}
10 changes: 5 additions & 5 deletions scripts/deploy.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ async function deployContract(wallet, contractName, args = []) {
return contract;
}

async function deployRemoteAddressValidator(wallet, interchainTokenServiceAddress, chainName) {
const remoteAddressValidatorImpl = await deployContract(wallet, 'RemoteAddressValidator', [interchainTokenServiceAddress, chainName]);
const params = defaultAbiCoder.encode(['string[]', 'string[]'], [[], []]);
async function deployRemoteAddressValidator(wallet, chainName, interchainTokenServiceAddress = '', evmChains = []) {
const remoteAddressValidatorImpl = await deployContract(wallet, 'RemoteAddressValidator', [chainName]);
const params = defaultAbiCoder.encode(['string[]', 'string[]'], [evmChains, evmChains.map(() => interchainTokenServiceAddress)]);

const remoteAddressValidatorProxy = await deployContract(wallet, 'RemoteAddressValidatorProxy', [
remoteAddressValidatorImpl.address,
Expand Down Expand Up @@ -74,15 +74,15 @@ async function deployTokenManagerImplementations(wallet, interchainTokenServiceA
return implementations;
}

async function deployAll(wallet, chainName, deploymentKey = 'interchainTokenService') {
async function deployAll(wallet, chainName, evmChains = [], deploymentKey = 'interchainTokenService') {
const create3Deployer = await deployContract(wallet, 'Create3Deployer');
const gateway = await deployMockGateway(wallet);
const gasService = await deployGasService(wallet);
const tokenManagerDeployer = await deployContract(wallet, 'TokenManagerDeployer', []);
const standardizedToken = await deployContract(wallet, 'StandardizedToken');
const standardizedTokenDeployer = await deployContract(wallet, 'StandardizedTokenDeployer', [standardizedToken.address]);
const interchainTokenServiceAddress = await getCreate3Address(create3Deployer.address, wallet, deploymentKey);
const remoteAddressValidator = await deployRemoteAddressValidator(wallet, interchainTokenServiceAddress, chainName);
const remoteAddressValidator = await deployRemoteAddressValidator(wallet, chainName, interchainTokenServiceAddress, evmChains);
const tokenManagerImplementations = await deployTokenManagerImplementations(wallet, interchainTokenServiceAddress);

const service = await deployInterchainTokenService(
Expand Down
48 changes: 15 additions & 33 deletions test/RemoteAddressValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ require('dotenv').config();
const chai = require('chai');
const { ethers } = require('hardhat');
const {
constants: { AddressZero },
utils: { defaultAbiCoder, keccak256, toUtf8Bytes },
utils: { defaultAbiCoder },
} = ethers;
const { expect } = chai;
const { deployRemoteAddressValidator, deployContract } = require('../scripts/deploy');
Expand All @@ -21,53 +20,33 @@ describe('RemoteAddressValidator', () => {
ownerWallet = wallets[0];
otherWallet = wallets[1];
interchainTokenServiceAddress = wallets[2].address;
remoteAddressValidator = await deployRemoteAddressValidator(ownerWallet, interchainTokenServiceAddress, chainName);
});

it('Should revert on RemoteAddressValidator deployment with invalid interchain token service address', async () => {
const remoteAddressValidatorFactory = await ethers.getContractFactory('RemoteAddressValidator');
await expect(remoteAddressValidatorFactory.deploy(AddressZero, chainName)).to.be.revertedWithCustomError(
remoteAddressValidator,
'ZeroAddress',
);
remoteAddressValidator = await deployRemoteAddressValidator(ownerWallet, chainName);
});

it('Should revert on RemoteAddressValidator deployment with invalid chain name', async () => {
const remoteAddressValidatorFactory = await ethers.getContractFactory('RemoteAddressValidator');
await expect(remoteAddressValidatorFactory.deploy(interchainTokenServiceAddress, '')).to.be.revertedWithCustomError(
remoteAddressValidator,
'ZeroStringLength',
);
await expect(remoteAddressValidatorFactory.deploy('')).to.be.revertedWithCustomError(remoteAddressValidator, 'ZeroStringLength');
});

it('Should revert on RemoteAddressValidator deployment with length mismatch between chains and trusted addresses arrays', async () => {
const remoteAddressValidatorImpl = await deployContract(ownerWallet, 'RemoteAddressValidator', [
interchainTokenServiceAddress,
chainName,
]);
const remoteAddressValidatorImpl = await deployContract(ownerWallet, 'RemoteAddressValidator', [chainName]);
const remoteAddressValidatorProxyFactory = await ethers.getContractFactory('RemoteAddressValidatorProxy');
const params = defaultAbiCoder.encode(['string[]', 'string[]'], [['Chain A'], []]);
await expect(
remoteAddressValidatorProxyFactory.deploy(remoteAddressValidatorImpl.address, ownerWallet.address, params),
).to.be.revertedWithCustomError(remoteAddressValidator, 'SetupFailed');
});

it('Should get the correct remote address for unregistered chains', async () => {
const remoteAddress = await remoteAddressValidator.getRemoteAddress(otherChain);
expect(remoteAddress).to.equal(interchainTokenServiceAddress.toLowerCase());
});

it('Should get the correct interchain token service address', async () => {
const itsAddress = await remoteAddressValidator.interchainTokenServiceAddress();
expect(itsAddress).to.equal(interchainTokenServiceAddress);

const itsAddressHash = await remoteAddressValidator.interchainTokenServiceAddressHash();
expect(itsAddressHash).to.equal(keccak256(toUtf8Bytes(interchainTokenServiceAddress.toLowerCase())));
it('Should revert when querrying the remote address for unregistered chains', async () => {
await expect(remoteAddressValidator.getRemoteAddress(otherChain)).to.be.revertedWithCustomError(
remoteAddressValidator,
'UntrustedChain',
);
});

it('Should be able to validate remote addresses properly', async () => {
expect(await remoteAddressValidator.validateSender(otherChain, otherRemoteAddress)).to.equal(false);
expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(true);
expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(false);
});

it('Should not be able to add a custom remote address as not the owner', async () => {
Expand Down Expand Up @@ -112,7 +91,10 @@ describe('RemoteAddressValidator', () => {
await expect(remoteAddressValidator.removeTrustedAddress(otherChain))
.to.emit(remoteAddressValidator, 'TrustedAddressRemoved')
.withArgs(otherChain);
expect(await remoteAddressValidator.getRemoteAddress(otherChain)).to.equal(interchainTokenServiceAddress.toLowerCase());
await expect(remoteAddressValidator.getRemoteAddress(otherChain)).to.be.revertedWithCustomError(
remoteAddressValidator,
'UntrustedChain',
);
});

it('Should revert on removing a custom remote address with an empty chain name', async () => {
Expand All @@ -124,6 +106,6 @@ describe('RemoteAddressValidator', () => {

it('Should be able to validate remote addresses properly.', async () => {
expect(await remoteAddressValidator.validateSender(otherChain, otherRemoteAddress)).to.equal(false);
expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(true);
expect(await remoteAddressValidator.validateSender(otherChain, interchainTokenServiceAddress)).to.equal(false);
});
});
Loading

0 comments on commit 111fed9

Please sign in to comment.