Skip to content

Commit

Permalink
feat(protocol): fix vault name and symbol validation bug with more un…
Browse files Browse the repository at this point in the history 
…it tests (#17013)

Co-authored-by: Keszey Dániel <[email protected]>
  • Loading branch information
@adaki2004
adaki2004 and Keszey Dániel authored May 7, 2024
1 parent bbcfb2d commit 8532b77
Show file tree
Hide file tree
Showing 4 changed files with 205 additions and 18 deletions.
2 changes: 1 addition & 1 deletion packages/protocol/contracts/bridge/Bridge.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ contract Bridge is EssentialContract, IBridge {
// Check if the destination chain is enabled.
(bool destChainEnabled,) = isDestChainEnabled(_message.destChainId);

// Verify destination chain and to address.
// Verify destination chain.
if (!destChainEnabled) revert B_INVALID_CHAINID();

// Ensure the sent value matches the expected amount.
Expand Down
2 changes: 1 addition & 1 deletion packages/protocol/contracts/tokenvault/BridgedERC20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ contract BridgedERC20 is EssentialContract, IBridgedERC20, ERC20Upgradeable {
initializer
{
// Check if provided parameters are valid
LibBridgedToken.validateInputs(_srcToken, _srcChainId, _symbol, _name);
LibBridgedToken.validateInputs(_srcToken, _srcChainId);
__Essential_init(_owner, _addressManager);
__ERC20_init(_name, _symbol);

Expand Down
15 changes: 0 additions & 15 deletions packages/protocol/contracts/tokenvault/LibBridgedToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,6 @@ library LibBridgedToken {
}
}

function validateInputs(
address _srcToken,
uint256 _srcChainId,
string memory _symbol,
string memory _name
)
internal
view
{
validateInputs(_srcToken, _srcChainId);
if (bytes(_symbol).length == 0 || bytes(_name).length == 0) {
revert BTOKEN_INVALID_PARAMS();
}
}

function checkToAddress(address _to) internal view {
if (_to == address(this)) revert BTOKEN_INVALID_TO_ADDR();
}
Expand Down
204 changes: 203 additions & 1 deletion packages/protocol/test/tokenvault/ERC20Vault.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,14 @@ contract TestERC20Vault is TaikoTest {
PrankDestBridge destChainIdBridge;
SkipProofCheckSignal mockProofSignalService;
FreeMintERC20 erc20;
FreeMintERC20 weirdNamedToken;
uint64 destChainId = 7;
uint64 srcChainId = uint64(block.chainid);

BridgedERC20 usdc;
BridgedERC20 usdt;
BridgedERC20 stETH;

function setUp() public {
vm.startPrank(Carol);
vm.deal(Alice, 1 ether);
Expand Down Expand Up @@ -122,6 +127,9 @@ contract TestERC20Vault is TaikoTest {
erc20 = new FreeMintERC20("ERC20", "ERC20");
erc20.mint(Alice);

weirdNamedToken = new FreeMintERC20("", "123456abcdefgh");
weirdNamedToken.mint(Alice);

bridge = Bridge(
payable(
deployProxy({
Expand Down Expand Up @@ -162,12 +170,51 @@ contract TestERC20Vault is TaikoTest {

addressManager.setAddress(uint64(block.chainid), "bridged_erc20", bridgedERC20);

usdc = BridgedERC20(
deployProxy({
name: "usdc",
impl: address(new BridgedERC20()),
data: abi.encodeCall(
BridgedERC20.init,
(address(0), address(addressManager), randAddress(), 100, 18, "USDC", "USDC coin")
)
})
);

usdt = BridgedERC20(
deployProxy({
name: "usdt",
impl: address(new BridgedERC20()),
data: abi.encodeCall(
BridgedERC20.init,
(address(0), address(addressManager), randAddress(), 100, 18, "USDT", "USDT coin")
)
})
);

stETH = BridgedERC20(
deployProxy({
name: "stETH",
impl: address(new BridgedERC20()),
data: abi.encodeCall(
BridgedERC20.init,
(
address(0),
address(addressManager),
randAddress(),
100,
18,
"stETH",
"Lido Staked ETH"
)
)
})
);
vm.stopPrank();
}

function test_20Vault_send_erc20_revert_if_allowance_not_set() public {
vm.startPrank(Alice);

vm.expectRevert("ERC20: insufficient allowance");
erc20Vault.sendToken(
ERC20Vault.BridgeTransferOp(
Expand Down Expand Up @@ -383,6 +430,16 @@ contract TestERC20Vault is TaikoTest {
});
}

function noNameErc20(uint64 chainId) internal view returns (ERC20Vault.CanonicalERC20 memory) {
return ERC20Vault.CanonicalERC20({
chainId: chainId,
addr: address(weirdNamedToken),
decimals: weirdNamedToken.decimals(),
symbol: weirdNamedToken.symbol(),
name: weirdNamedToken.name()
});
}

function test_20Vault_upgrade_bridged_tokens_20() public {
vm.startPrank(Alice);

Expand Down Expand Up @@ -463,4 +520,149 @@ contract TestERC20Vault is TaikoTest {
assertEq(aliceBalanceAfterRecall, aliceBalanceBefore);
assertEq(erc20VaultBalanceAfterRecall, erc20VaultBalanceBefore);
}

function test_20Vault_change_bridged_token() public {
// A mock canonical "token"
address canonicalRandomToken = vm.addr(102);

vm.warp(block.timestamp + 91 days);

vm.startPrank(Carol);

erc20Vault.changeBridgedToken(
ERC20Vault.CanonicalERC20({
chainId: 1,
addr: address(erc20),
decimals: 18,
symbol: "ERC20TT",
name: "ERC20 Test token"
}),
address(usdc)
);

assertEq(erc20Vault.canonicalToBridged(1, address(erc20)), address(usdc));

vm.expectRevert(ERC20Vault.VAULT_LAST_MIGRATION_TOO_CLOSE.selector);
erc20Vault.changeBridgedToken(
ERC20Vault.CanonicalERC20({
chainId: 1,
addr: address(erc20),
decimals: 18,
symbol: "ERC20TT",
name: "ERC20 Test token"
}),
address(usdt)
);

vm.warp(block.timestamp + 91 days);

vm.expectRevert(ERC20Vault.VAULT_CTOKEN_MISMATCH.selector);
erc20Vault.changeBridgedToken(
ERC20Vault.CanonicalERC20({
chainId: 1,
addr: address(erc20),
decimals: 18,
symbol: "ERC20TT_WRONG_NAME",
name: "ERC20 Test token"
}),
address(usdt)
);

erc20Vault.changeBridgedToken(
ERC20Vault.CanonicalERC20({
chainId: 1,
addr: address(erc20),
decimals: 18,
symbol: "ERC20TT",
name: "ERC20 Test token"
}),
address(usdt)
);

assertEq(erc20Vault.canonicalToBridged(1, address(erc20)), address(usdt));

erc20Vault.changeBridgedToken(
ERC20Vault.CanonicalERC20({
chainId: 1,
addr: canonicalRandomToken,
decimals: 18,
symbol: "ERC20TT2",
name: "ERC20 Test token2"
}),
address(stETH)
);

vm.warp(block.timestamp + 91 days);

// usdc is already blacklisted!
vm.expectRevert(ERC20Vault.VAULT_BTOKEN_BLACKLISTED.selector);
erc20Vault.changeBridgedToken(
ERC20Vault.CanonicalERC20({
chainId: 1,
addr: address(erc20),
decimals: 18,
symbol: "ERC20TT",
name: "ERC20 Test token"
}),
address(usdc)
);

// We cannot use stETH for erc20 (as it is used in connection with another token)
vm.expectRevert(ERC20Vault.VAULT_INVALID_NEW_BTOKEN.selector);
erc20Vault.changeBridgedToken(
ERC20Vault.CanonicalERC20({
chainId: 1,
addr: address(erc20),
decimals: 18,
symbol: "ERC20TT",
name: "ERC20 Test token"
}),
address(stETH)
);

vm.stopPrank();
}

function test_20Vault_to_string() public {
vm.startPrank(Alice);

(, bytes memory symbolData) =
address(weirdNamedToken).staticcall(abi.encodeCall(INameSymbol.symbol, ()));
(, bytes memory nameData) =
address(weirdNamedToken).staticcall(abi.encodeCall(INameSymbol.name, ()));

string memory decodedSymbol = LibBytes.toString(symbolData);
string memory decodedName = LibBytes.toString(nameData);

assertEq(decodedSymbol, "123456abcdefgh");
assertEq(decodedName, "");

vm.stopPrank();
}

function test_20Vault_deploy_erc20_with_no_name() public {
vm.startPrank(Alice);

vm.chainId(destChainId);

uint64 amount = 1;

destChainIdBridge.setERC20Vault(address(destChainIdERC20Vault));

address bridgedAddressBefore =
destChainIdERC20Vault.canonicalToBridged(srcChainId, address(erc20));
assertEq(bridgedAddressBefore == address(0), true);

// Token with empty name succeeds
destChainIdBridge.sendReceiveERC20ToERC20Vault(
noNameErc20(srcChainId),
Alice,
Bob,
amount,
bytes32(0),
address(erc20Vault),
srcChainId,
0
);
}
}

0 comments on commit 8532b77

Please sign in to comment.