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.

Sign up for GitHub

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

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

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6be8d2e
add changeBridgedToken and toString tests
May 6, 2024
610231e
check forge formatting error
May 6, 2024
1274de8
test with empty name/symbol
May 6, 2024
6276ffd
remove name and symbol validation
May 7, 2024
2fa2fe1
Merge branch 'main' into more_vault_tests
adaki2004 May 7, 2024
0d741cf
fix commenting
May 7, 2024
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
217 changes: 216 additions & 1 deletion packages/protocol/test/tokenvault/ERC20Vault.t.sol
View file
Open in desktop
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,162 @@ contract TestERC20Vault is TaikoTest {
assertEq(aliceBalanceAfterRecall, aliceBalanceBefore);
assertEq(erc20VaultBalanceAfterRecall, erc20VaultBalanceBefore);
}

function test_20Vault_change_bridged_token() public {
// Other canonical - bridge token part cannot assigned
address canonicalRandomToken = vm.addr(102);
address bridgedForCanonical = vm.addr(103);

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);

// todo: dantaik
// So this token (weirdNamedToken) has no name (""), as name() and symbol() is optional. And
// there is an issue with that.
// The following 2 scenarios would create a revert on the "destination" chain:
// A: when there is no name (also applies to symbol), we will use that exact same empty
// string decoded "" (LibBytes()).
// B: if the name() (also applies to symbol) low level call would fail, but we also use
// default "" (Base vault line 78).
//
// In these 2 scenarios, the LibBridgedToken.validateInputs(_srcToken, _srcChainId, _symbol,
// _name) -> Would fail.
// Either do not invalidate on empty string OR use other than "" as an error default retVal
// (in LibBytes) or default value.
destChainIdBridge.sendReceiveERC20ToERC20Vault(
noNameErc20(srcChainId),
Alice,
Bob,
amount,
bytes32(0),
address(erc20Vault),
srcChainId,
0
);
}
}
Loading