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.

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

fix(protocol): fix vault name and symbol validation bug with more unit tests #17013

Merged
merged 6 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/protocol/contracts/bridge/Bridge.sol
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
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
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
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
);
}
}