Skip to content

Commit

Permalink
(feat) A LockProxyProxy contract which limits access to the LockProxy…
Browse files Browse the repository at this point in the history
… extension to nominated tokens.
  • Loading branch information
rrw-zilliqa committed Dec 6, 2024
1 parent d251a5d commit 7793dcb
Show file tree
Hide file tree
Showing 13 changed files with 252 additions and 57 deletions.
15 changes: 10 additions & 5 deletions docs/zilbridge.md
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,6 @@ forge script script/bsc-testnet/deployXBridgeOverMockZilBridge.s.sol \
--rpc-url https://bsc-testnet.bnbchain.org --broadcast
forge verify-contract <address> --rpc-url https://bsc-testnet.bnbchain.org \
--chain-id 97
# and again ..
forge script scripts/bsc-testnet/deployZilBridgeTokenManagers.s.sol \
--rpc-url https://bsc-testnet.bnbchain.org --broadcast
forge verify-contract <address> --rpc-url https://bsc-testnet.bnbchain.org \
--chain-id 97

```

Expand Down Expand Up @@ -150,6 +145,16 @@ Now we can deploy some contracts to the BNB testnet:
forge script script/bsc-testnet/deployZilBridgeTokens.s.sol \
--tc Deployment --rpc-url https://bsc-testnet.bnbchain.org \
--broadcast
```

And, once we've recorded the token addresses (so that the
`LockProxyProxy` knows about them):

```

Check notice on line 153 in docs/zilbridge.md

View workflow job for this annotation

GitHub Actions / Trunk Check

markdownlint(MD040)

[new] Fenced code blocks should have a language specified
forge script scripts/bsc-testnet/deployZilBridgeTokenManagers.s.sol \
--rpc-url https://bsc-testnet.bnbchain.org --broadcast
forge verify-contract <address> --rpc-url https://bsc-testnet.bnbchain.org \
--chain-id 97
forge verify-contract <address> --rpc-url https://bsc-testnet.bnbchain.org \
--chain-id 97
```
Expand Down
8 changes: 8 additions & 0 deletions smart-contracts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,14 @@ classDiagram
}
```

## Testing

When testing, you'll need to artificially raise the gas limit for `forge test` to make the `largeValidatorSet()` tests run:

Check notice on line 102 in smart-contracts/README.md

View workflow job for this annotation

GitHub Actions / Trunk Check

markdownlint(MD013)

[new] Line length

```sh
forge test --gas-limit 2000000000000
```

## Deployments

### Core
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity ^0.8.20;

// The part of the LockProxy extension interface that allows us to mint and burn
// tokens.
interface ILockProxyExtensionTransfer {
function extensionTransfer(
address _receivingAddress,
address _assetHash,
uint256 _amount
) external returns (bool);
}
51 changes: 51 additions & 0 deletions smart-contracts/contracts/periphery/LockProxyProxy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity ^0.8.20;

import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { Ownable2Step } from "@openzeppelin/contracts/access/Ownable2Step.sol";
import { EnumerableSet } from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import { ILockProxyExtensionTransfer } from "contracts/periphery/ILockProxyExtensionTransfer.sol";

/// @dev This is a lock proxy proxy; it ensures that the extension is only called for a fixed set of tokens,
/// decided at deployment time.
contract LockProxyProxy is ILockProxyExtensionTransfer, Ownable2Step {
using EnumerableSet for EnumerableSet.AddressSet;

address public lockProxy;
mapping(address => bool) legalTokens;
EnumerableSet.AddressSet legalCallers;

error UnauthorizedCaller(address caller);

constructor(address[] memory _tokens, address _owner, address _lockProxy) Ownable(_owner) {
lockProxy = _lockProxy;
for (uint i; i < _tokens.length; ++i) {
legalTokens[_tokens[i]] = true;
}
}

function addCaller(address _caller) onlyOwner public {
legalCallers.add(_caller);

}

function removeCaller(address _caller) onlyOwner public {
legalCallers.remove(_caller);
}

function enumerateCallers() public view returns (address[] memory) {
return legalCallers.values();
}

modifier onlyValidCaller() {
if (!legalCallers.contains(_msgSender())) {
revert UnauthorizedCaller(_msgSender());
}
_;
}

function extensionTransfer(address _receivingAddress, address _assetHash, uint256 _amount) external onlyValidCaller returns (bool) {
require(legalTokens[_assetHash], "Bad asset hash");
return ILockProxyExtensionTransfer(lockProxy).extensionTransfer(_receivingAddress, _assetHash, _amount);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ pragma solidity ^0.8.20;
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";

interface ILockProxyTokenManagerStorage {
event LockProxyUpdated(address oldLockProxy, address newLockProxy);
event LockProxyStorageUpdated(address oldLockProxy, address newLockProxy, address oldLockProxyProxy, address newLockProxyProxy);
function getLockProxy() external view returns (address);
function setLockProxy(address lockProxy) external;
function getLockProxyProxy() external view returns (address);
function setLockProxyData(address lockProxy, address lockProxyProxy) external;
}

abstract contract LockProxyTokenManagerStorage is ILockProxyTokenManagerStorage {
/// @custom:storage-location erc7201:zilliqa.storage.LockProxyTokenManagerStorage
struct LockProxyTokenManagerStorageStruct {
address lockProxyProxy;
address lockProxy;
}

Expand All @@ -25,14 +27,21 @@ abstract contract LockProxyTokenManagerStorage is ILockProxyTokenManagerStorage
}
}

function getLockProxyProxy() public view returns (address) {
LockProxyTokenManagerStorageStruct storage $ = _getLockProxyTokenManagerStorageStruct();
return $.lockProxyProxy;
}

function getLockProxy() public view returns (address) {
LockProxyTokenManagerStorageStruct storage $ = _getLockProxyTokenManagerStorageStruct();
return $.lockProxy;
}

function _setLockProxy(address lockProxy) internal {
function _setLockProxyData(address lockProxy, address lockProxyProxy) internal {
LockProxyTokenManagerStorageStruct storage $ = _getLockProxyTokenManagerStorageStruct();
emit LockProxyUpdated($.lockProxy, lockProxy);
emit LockProxyStorageUpdated($.lockProxy, lockProxy, $.lockProxyProxy, lockProxyProxy);
$.lockProxy = lockProxy;
$.lockProxyProxy = lockProxyProxy;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface ILockProxyTokenManager is ITokenManager {
// Args in this order to match other token managers.
event SentToLockProxy(address indexed token, address indexed sender, uint amount);
event WithdrawnFromLockProxy(address indexed token, address indexed receipient, uint amount);
function setLockProxy(address lockProxy) external;
function setLockProxyData(address lockProxy, address lockProxyProxy) external;
}

// This contract exists almost entirely to be used in tests to prove upgradeability.
Expand All @@ -24,14 +24,14 @@ contract LockProxyTokenManagerUpgradeable is
}


function setLockProxy(address lockProxy) external override(ILockProxyTokenManagerStorage, ILockProxyTokenManager) onlyOwner {
_setLockProxy(lockProxy);
function setLockProxyData(address lockProxy, address lockProxyProxy) external override(ILockProxyTokenManagerStorage, ILockProxyTokenManager) onlyOwner {
_setLockProxyData(lockProxy, lockProxyProxy);
}

function initialize(address _gateway, address lockProxy) external initializer {
__TokenManager_init(_gateway);
// for some reason we can't call setLockProxy() here, so ..
_setLockProxy(lockProxy);
function initialize(address _gateway, address lockProxy, address lockProxyProxy) external initializer {
__TokenManager_init(_gateway);
// for some reason we can't call setLockProxy() here, so ..
_setLockProxyData(lockProxy, lockProxyProxy);
}

// Outgoing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,14 @@ import {TokenManagerUpgradeableV3, ITokenManager} from "contracts/periphery/Toke
import {BridgedToken} from "contracts/periphery/BridgedToken.sol";
import {IERC20} from "contracts/periphery/LockAndReleaseTokenManagerUpgradeable.sol";
import { ILockProxyTokenManagerStorage, LockProxyTokenManagerStorage } from "contracts/periphery/LockProxyTokenManagerStorage.sol";
import { ILockProxyExtensionTransfer } from "contracts/periphery/ILockProxyExtensionTransfer.sol";

interface ILockProxyTokenManager is ILockProxyTokenManagerStorage {
// Args in this order to match other token managers.
event SentToLockProxy(address indexed token, address indexed sender, uint amount);
event WithdrawnFromLockProxy(address indexed token, address indexed receipient, uint amount);
}

// Exists purely so that we can call the extensionTransfer() endpoint on the lock proxy without having
// to include its code here.
interface ILockProxyExtensionTransfer {
function extensionTransfer(
address _receivingAddress,
address _assetHash,
uint256 _amount
)
external
returns (bool);
}

// This is the lock proxy token manager that runs on EVM chains. It talks to an EVM LockProxy.
contract LockProxyTokenManagerUpgradeableV3 is TokenManagerUpgradeableV3, ILockProxyTokenManager, LockProxyTokenManagerStorage {
address public constant NATIVE_ASSET_HASH = address(0);
Expand All @@ -36,7 +25,7 @@ contract LockProxyTokenManagerUpgradeableV3 is TokenManagerUpgradeableV3, ILockP
_setFees(fees);
}

// Incoming currency - transfer into the lock proxy
// Incoming currency - transfer into the lock proxy (directly!)
function _handleTransfer(address token, address from, uint amount) internal override {
address lockProxyAddress = getLockProxy();
// Just transfer value to the lock proxy.
Expand All @@ -52,9 +41,11 @@ contract LockProxyTokenManagerUpgradeableV3 is TokenManagerUpgradeableV3, ILockP
emit SentToLockProxy(token, from, amount);
}

// Withdrawals are processed via the lockProxyProxy.
function _handleAccept(address token, address recipient, uint amount) internal override {
address lockProxyProxyAddress = getLockProxyProxy();
address lockProxyAddress = getLockProxy();
ILockProxyExtensionTransfer lp = ILockProxyExtensionTransfer(payable(lockProxyAddress));
ILockProxyExtensionTransfer lp = ILockProxyExtensionTransfer(payable(lockProxyProxyAddress));
// Sadly, extensionTransfer() takes the same arguments as the withdrawn event but in a
// different order. This will automagically transfer native token if token==0.

Expand All @@ -65,13 +56,15 @@ contract LockProxyTokenManagerUpgradeableV3 is TokenManagerUpgradeableV3, ILockP
} else {
lp.extensionTransfer(address(this), token, amount);
IERC20 erc20token = IERC20(token);
erc20token.transferFrom(address(lp), recipient, amount);
// Although the lockProxyProxy is the registered extension, the tokens are held by the actual
// lockProxy
erc20token.transferFrom(lockProxyAddress, recipient, amount);
}
emit WithdrawnFromLockProxy(token, recipient, amount);
}

function setLockProxy(address lockProxy) external onlyOwner {
_setLockProxy(lockProxy);
function setLockProxyData(address lockProxy, address lockProxyProxy) external onlyOwner {
_setLockProxyData(lockProxy, lockProxyProxy);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { EthCrossChainManager } from "test/zilbridge/infrastructure/ccmCrossChai
import { EthCrossChainData } from "test/zilbridge/infrastructure/ethCrossChainData.sol";
import { EthExtendCrossChainManager } from "contracts/periphery/ZilBridge/ccmExtendCrossChainManager.sol";
import { ChainGateway } from "contracts/core/ChainGateway.sol";
import { LockProxyProxy } from "contracts/periphery/LockProxyProxy.sol";
import { LockProxyTokenManagerUpgradeableV3 } from "contracts/periphery/TokenManagerV3/LockProxyTokenManagerUpgradeableV3.sol";
import { LockProxyTokenManagerDeployer } from "test/zilbridge/TokenManagerDeployers/LockProxyTokenManagerDeployer.sol";
import {LockProxyTokenManagerUpgradeableV3} from "contracts/periphery/TokenManagerV3/LockProxyTokenManagerUpgradeableV3.sol";
Expand Down Expand Up @@ -39,12 +40,18 @@ contract deployZilBridgeTokenManagers is Script, LockProxyTokenManagerDeployer,
uint256 bridgePrivateKey = vm.envUint("PRIVATE_KEY_ZILBRIDGE");
// token managers are apparently not pausable, so ..
vm.startBroadcast(validatorPrivateKey);
LockProxyTokenManagerUpgradeableV3 tokenManager = deployLatestLockProxyTokenManager(address(chainGateway), address(lockProxy), fees);
address[] memory allowedTokens = new address[](3);
allowedTokens[0] = bscERC20Address;
allowedTokens[1] = bscBridgedZRC2Address;
allowedTokens[2] = bscBridgedZILAddress;
LockProxyProxy lockProxyProxy = new LockProxyProxy(allowedTokens, vm.addr(validatorPrivateKey), address(lockProxy));
LockProxyTokenManagerUpgradeableV3 tokenManager = deployLatestLockProxyTokenManager(address(chainGateway), address(lockProxy), address(lockProxyProxy), fees);
lockProxyProxy.addCaller(address(tokenManager));
console.log(
" address public constant bscLockProxyTokenManagerAddress = %s", address(tokenManager));
vm.stopBroadcast();
vm.startBroadcast(bridgePrivateKey);
extendCCM.forciblyAddExtension(address(lockProxy), address(tokenManager), COUNTERPART_CHAIN_ID);
extendCCM.forciblyAddExtension(address(lockProxyProxy), address(tokenManager), COUNTERPART_CHAIN_ID);
vm.stopBroadcast();
vm.startBroadcast(validatorPrivateKey);
chainGateway.register(address(tokenManager));
Expand Down
82 changes: 82 additions & 0 deletions smart-contracts/test/periphery/LockProxyProxy.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
pragma solidity 0.8.20;

import {Tester, Vm} from "test/Tester.sol";
import { LockProxyProxy } from "contracts/periphery/LockProxyProxy.sol";
import { ILockProxyExtensionTransfer } from "contracts/periphery/ILockProxyExtensionTransfer.sol";
import { TestToken } from "test/Helpers.sol";

contract TestLockProxyProxy is Tester, ILockProxyExtensionTransfer {
Vm.Wallet validatorWallet = vm.createWallet("validator");
address validator = validatorWallet.addr;

Vm.Wallet callerWallet = vm.createWallet("caller");
Vm.Wallet otherWallet = vm.createWallet("other");

Vm.Wallet transferorWallet = vm.createWallet("transferor");

LockProxyProxy theProxy;
TestToken allowedToken;
TestToken disallowedToken;
uint tokenSupply = 1000 ether;
uint tokensTransferred = 12 ether;

function extensionTransfer(address _receivingAddress, address /* _assetHash */, uint256 _amount) external returns (bool) {
assertEq(_receivingAddress, transferorWallet.addr);
// Deliberately don't validate on the asset hash because we want the caller to do it.
assertEq(_amount, tokensTransferred);
return true;
}


function setUp() public {
vm.startPrank(validator);
// Deploy the test token
allowedToken = new TestToken(tokenSupply);
disallowedToken = new TestToken(tokenSupply);
address[] memory allowedTokens = new address[](1);
allowedTokens[0] = address(allowedToken);
theProxy = new LockProxyProxy(allowedTokens, validator, address(this));
theProxy.addCaller(callerWallet.addr);
vm.stopPrank();
}

function testTransfer() public {
vm.startPrank(callerWallet.addr);
ILockProxyExtensionTransfer(theProxy).extensionTransfer(transferorWallet.addr, address(allowedToken),
tokensTransferred );
vm.stopPrank();
}

function testInvalidToken() public {
vm.expectRevert("Bad asset hash");
vm.startPrank(callerWallet.addr);
ILockProxyExtensionTransfer(theProxy).extensionTransfer(transferorWallet.addr, address(disallowedToken), tokensTransferred);
vm.stopPrank();
}

function testInvalidCaller() public {
vm.expectRevert();
vm.startPrank(otherWallet.addr);
ILockProxyExtensionTransfer(theProxy).extensionTransfer(transferorWallet.addr, address(allowedToken), tokensTransferred);
vm.stopPrank();
}

function testRemoveCaller() public {
vm.startPrank(validator);
theProxy.removeCaller(callerWallet.addr);
vm.stopPrank();
vm.startPrank(callerWallet.addr);
vm.expectRevert();
ILockProxyExtensionTransfer(theProxy).extensionTransfer(transferorWallet.addr, address(allowedToken),
tokensTransferred );
vm.stopPrank();
vm.startPrank(validator);
theProxy.addCaller(callerWallet.addr);
vm.stopPrank();
vm.startPrank(callerWallet.addr);
ILockProxyExtensionTransfer(theProxy).extensionTransfer(transferorWallet.addr, address(allowedToken),
tokensTransferred );
vm.stopPrank();
}
}
8 changes: 3 additions & 5 deletions smart-contracts/test/zilbridge/DeployZilBridge.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ abstract contract ZilBridgeFixture is Tester, LockProxyTokenManagerDeployer {
installExtendCrossChainManager(owner);
}

function installTokenManager(address lpTokenManager) internal {
function installLockProxyProxy(address lockProxyProxy) internal {
// Make it an extension
vm.startPrank(owner);
extendCCM.forciblyAddExtension(address(lockProxy), address(lpTokenManager), COUNTERPART_CHAIN_ID);
extendCCM.forciblyAddExtension(address(lockProxy), address(lockProxyProxy), COUNTERPART_CHAIN_ID);
vm.stopPrank();
}

Expand Down Expand Up @@ -115,9 +115,7 @@ contract DeployZilBridgeTest is ZilBridgeFixture {
require(!ccmProxy.paused());
}



function test_installTokenManager() external {
function test_installExtendedCCM() external {
deployOriginalContracts();
installExtendCrossChainManager(owner);
}
Expand Down
Loading

0 comments on commit 7793dcb

Please sign in to comment.