fix(bridge): Token Vault sendEther messages with processing fees are …

…impossible to send (#277) * tokenVault deposit value should be msg.value - processingFee for sendEther * test names * Update TokenVault.sol * dup require Co-authored-by: dantaik <99078276+dantaik@users.noreply.github.com>
taikoxyz · Nov 18, 2022 · 10d9bbc · 10d9bbc
1 parent b1a10b0
commit 10d9bbc
Show file tree

Hide file tree

Showing 3 changed files with 174 additions and 3 deletions.
diff --git a/packages/protocol/contracts/bridge/TokenVault.sol b/packages/protocol/contracts/bridge/TokenVault.sol
@@ -122,7 +122,7 @@ contract TokenVault is EssentialContract {
             to != address(0) && to != resolve(destChainId, "token_vault"),
             "V:to"
         );
-        require(msg.value > 0, "V:msgValue");
+        require(msg.value > processingFee, "V:msgValue");
 
         IBridge.Message memory message;
         message.destChainId = destChainId;
@@ -131,7 +131,7 @@ contract TokenVault is EssentialContract {
 
         message.gasLimit = gasLimit;
         message.processingFee = processingFee;
-        message.depositValue = msg.value;
+        message.depositValue = msg.value - processingFee;
         message.refundAddress = refundAddress;
         message.memo = memo;
 
@@ -141,7 +141,7 @@ contract TokenVault is EssentialContract {
             value: msg.value
         }(message);
 
-        emit EtherSent(to, destChainId, msg.value, signal);
+        emit EtherSent(to, destChainId, message.depositValue, signal);
     }
 
     /**

diff --git a/packages/protocol/contracts/test/thirdparty/TestMessageSender.sol b/packages/protocol/contracts/test/thirdparty/TestMessageSender.sol
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.9;
+
+import "../../bridge/IBridge.sol";
+
+contract TestMessageSender {
+
+    bytes32 signal = 0x3fd54831f488a22b28398de0c567a3b064b937f54f81739ae9bd545967f3abab;
+
+    function sendMessage(
+        IBridge.Message calldata message
+    ) external payable returns (bytes32) {
+        message;
+        return signal;
+    }
+}
diff --git a/packages/protocol/test/bridge/TokenVault.test.ts b/packages/protocol/test/bridge/TokenVault.test.ts
@@ -38,6 +38,18 @@ describe("TokenVault", function () {
 
         await tokenVault.init(tokenVaultAddressManager.address)
 
+        const network = await ethers.provider.getNetwork()
+
+        const TestMessageSenderFactory = await ethers.getContractFactory(
+            "TestMessageSender"
+        )
+
+        const testMessageSender = await TestMessageSenderFactory.deploy()
+
+        await tokenVaultAddressManager.setAddress(
+            `${network.chainId}.bridge`,
+            testMessageSender.address
+        )
         return {
             owner,
             nonOwner,
@@ -62,4 +74,147 @@ describe("TokenVault", function () {
             ).to.be.revertedWith(ADDRESS_RESOLVER_DENIED)
         })
     })
+
+    describe("sendEther()", async () => {
+        it("throws when msg.value is 0", async () => {
+            const { owner, tokenVault } = await deployTokenVaultFixture()
+
+            const processingFee = 10
+
+            await expect(
+                tokenVault.sendEther(
+                    167001,
+                    owner.address,
+                    10000,
+                    processingFee,
+                    owner.address,
+                    ""
+                )
+            ).to.be.revertedWith("V:msgValue")
+        })
+
+        it("throws when msg.value - processing fee is 0", async () => {
+            const { owner, tokenVault } = await deployTokenVaultFixture()
+
+            const processingFee = 10
+
+            await expect(
+                tokenVault.sendEther(
+                    167001,
+                    owner.address,
+                    10000,
+                    processingFee,
+                    owner.address,
+                    "",
+                    {
+                        value: processingFee,
+                    }
+                )
+            ).to.be.revertedWith("V:msgValue")
+        })
+
+        it("throws when msg.value is < processingFee", async () => {
+            const { owner, tokenVault } = await deployTokenVaultFixture()
+
+            const processingFee = 10
+
+            await expect(
+                tokenVault.sendEther(
+                    167001,
+                    owner.address,
+                    10000,
+                    processingFee,
+                    owner.address,
+                    "",
+                    {
+                        value: processingFee - 1,
+                    }
+                )
+            ).to.be.revertedWith("V:msgValue")
+        })
+
+        it("throws when to is 0", async () => {
+            const { owner, tokenVault } = await deployTokenVaultFixture()
+
+            const processingFee = 10
+
+            await expect(
+                tokenVault.sendEther(
+                    167001,
+                    ethers.constants.AddressZero,
+                    10000,
+                    processingFee,
+                    owner.address,
+                    "",
+                    {
+                        value: processingFee - 1,
+                    }
+                )
+            ).to.be.revertedWith("V:to")
+        })
+
+        it("succeeds with processingFee", async () => {
+            const { owner, tokenVault } = await deployTokenVaultFixture()
+
+            const processingFee = 10
+            const depositValue = 1000
+            const destChainId = 167001
+
+            const testSignal =
+                "0x3fd54831f488a22b28398de0c567a3b064b937f54f81739ae9bd545967f3abab"
+
+            await expect(
+                tokenVault.sendEther(
+                    destChainId,
+                    owner.address,
+                    10000,
+                    processingFee,
+                    owner.address,
+                    "",
+                    {
+                        value: depositValue,
+                    }
+                )
+            )
+                .to.emit(tokenVault, "EtherSent")
+                .withArgs(
+                    owner.address,
+                    destChainId,
+                    depositValue - processingFee,
+                    testSignal
+                )
+        })
+
+        it("succeeds with 0 processingFee", async () => {
+            const { owner, tokenVault } = await deployTokenVaultFixture()
+
+            const processingFee = 0
+            const depositValue = 1000
+            const destChainId = 167001
+
+            const testSignal =
+                "0x3fd54831f488a22b28398de0c567a3b064b937f54f81739ae9bd545967f3abab"
+
+            await expect(
+                tokenVault.sendEther(
+                    destChainId,
+                    owner.address,
+                    10000,
+                    processingFee,
+                    owner.address,
+                    "",
+                    {
+                        value: depositValue,
+                    }
+                )
+            )
+                .to.emit(tokenVault, "EtherSent")
+                .withArgs(
+                    owner.address,
+                    destChainId,
+                    depositValue - processingFee,
+                    testSignal
+                )
+        })
+    })
 })