harmony-one#95 added cancelReplace, test case, snapshot, revert

* ensure we request above dust amount (non-zero)

contract/bridge/contracts/OneBtc.sol

@@ -184,6 +184,11 @@ contract OneBtc is ERC20Upgradeable, Issue, Redeem, Replace {
         );
     }
 
+    function cancelReplace(address payable oldVaultId,
+        uint256 btcAmount) external {
+        Replace._withdrawReplace(oldVaultId, btcAmount);
+    }
+
     function executeReplace(
         uint256 replaceId,
         bytes calldata merkleProof,
@@ -197,41 +202,41 @@ contract OneBtc is ERC20Upgradeable, Issue, Redeem, Replace {
         Replace._executeReplace(replaceId, _vout);
     }
 
-    /**
-     * @dev Report vault misbehavior by providing fraud proof (malicious bitcoin transaction and the corresponding transaction inclusion proof). Fully slashes the vault.
-     */
-    function reportVaultTheft(
-        address vaultId,
-        bytes calldata rawTx,
-        uint32 height,
-        uint256 index,
-        bytes calldata merkleProof,
-        bytes calldata header
-    ) external {
-        require(
-            relay.isApprovedStakedRelayer(msg.sender),
-            "Sender is not authorized"
-        );
+    // /**
+    //  * @dev Report vault misbehavior by providing fraud proof (malicious bitcoin transaction and the corresponding transaction inclusion proof). Fully slashes the vault.
+    //  */
+    // function reportVaultTheft(
+    //     address vaultId,
+    //     bytes calldata rawTx,
+    //     uint32 height,
+    //     uint256 index,
+    //     bytes calldata merkleProof,
+    //     bytes calldata header
+    // ) external {
+    //     require(
+    //         relay.isApprovedStakedRelayer(msg.sender),
+    //         "Sender is not authorized"
+    //     );
 
-        bytes32 txId = rawTx.hash256();
+    //     bytes32 txId = rawTx.hash256();
 
-        // check if already reported
-        bytes32 reportKey = keccak256(abi.encodePacked(vaultId, txId));
-        require(
-            theftReports[reportKey] == false,
-            "This txId has already been logged as a theft by the given vault"
-        );
+    //     // check if already reported
+    //     bytes32 reportKey = keccak256(abi.encodePacked(vaultId, txId));
+    //     require(
+    //         theftReports[reportKey] == false,
+    //         "This txId has already been logged as a theft by the given vault"
+    //     );
 
-        // verify transaction inclusion using header and merkle proof
-        relay.verifyTx(height, index, txId, header, merkleProof, 1, true);
+    //     // verify transaction inclusion using header and merkle proof
+    //     relay.verifyTx(height, index, txId, header, merkleProof, 1, true);
 
-        // all looks good, liquidate vault
-        address reporterId = msg.sender;
-        liquidateVault(vaultId, reporterId);
+    //     // all looks good, liquidate vault
+    //     address reporterId = msg.sender;
+    //     liquidateVault(vaultId, reporterId);
 
-        theftReports[reportKey] = true;
-        emit ReportVaultTheft(vaultId);
-    }
+    //     theftReports[reportKey] = true;
+    //     emit ReportVaultTheft(vaultId);
+    // }
 
     /**
      * @dev Reports vault double payment providing two fraud proof (malicious bitcoin transaction and the corresponding transaction inclusion proof). Fully slashes the vault.

contract/bridge/contracts/Replace.sol

@@ -96,7 +96,7 @@ abstract contract Replace is VaultRegistry, Request {
         // a request with amount=0 is valid, as long the _total_ is above DUST. This might
         // be the case if the vault just wants to increase the griefing collateral, for example.
         uint256 dustValue = 0;
-        require(totalToBeReplaced >= dustValue, "Amount below dust amount");
+        require(totalToBeReplaced > dustValue, "Amount below dust amount");
 
         // check that that the total griefing collateral is sufficient to back the total to-be-replaced amount
         require(
@@ -118,13 +118,14 @@ abstract contract Replace is VaultRegistry, Request {
     function _withdrawReplace(address oldVaultId, uint256 btcAmount) internal {
         require(msg.sender == oldVaultId, "Sender should be old vault owner");
         // TODO: SECURITY CHECK (The oldVault MUST NOT be banned)
+        // already checked by requestReplace, unless we care about that small window.
 
         (uint256 withdrawnTokens, uint256 toWithdrawCollateral) = VaultRegistry
             .decreaseToBeReplacedTokens(oldVaultId, btcAmount);
 
         ICollateral.releaseCollateral(oldVaultId, toWithdrawCollateral);
 
-        require(withdrawnTokens == 0, "Withdraw tokens is zero");
+        require(withdrawnTokens != 0, "Withdraw tokens is zero");
 
         emit WithdrawReplace(oldVaultId, withdrawnTokens, toWithdrawCollateral);
     }

contract/bridge/contracts/VaultRegistry.sol

@@ -256,14 +256,14 @@ abstract contract VaultRegistry is Initializable, ICollateral {
         return (vault.toBeReplaced, vault.replaceCollateral);
     }
 
-    function decreaseToBeReplacedTokens(address vaultId, uint256 tokens)
+    function decreaseToBeReplacedTokens(address vaultId, uint256 btcAmount)
         internal
         returns (uint256, uint256)
     {
         Vault storage vault = vaults[vaultId];
         require(vault.btcPublicKeyX != 0, "Vault does not exist");
 
-        uint256 usedTokens = MathUpgradeable.min(vault.toBeReplaced, tokens);
+        uint256 usedTokens = MathUpgradeable.min(vault.toBeReplaced, btcAmount);
 
         uint256 calculatedCollateral = calculateCollateral(
             vault.replaceCollateral,

contract/bridge/test/Replace.js

@@ -22,6 +22,16 @@ web3.extend({
       call: "evm_mine",
       params: 0,
     },
+    {
+      name: "snapshot",
+      call: "evm_snapshot",
+      params: 0,
+    },
+    {
+      name: "revert",
+      call: "evm_revert",
+      params: 1,
+    },         
   ],
 });
 
@@ -122,6 +132,10 @@ contract("Replace unit test", (accounts) => {
       griefingCollateral.toString(),
       IssueEvent.args.griefingCollateral.toString()
     );
+
+    // snapshot so we can test cancel later
+    this.snapshotId = await web3.miner.snapshot();
+
   });
 
   it("Register NEW Vault with 10 Wei Collateral", async function () {
@@ -201,4 +215,24 @@ contract("Replace unit test", (accounts) => {
     assert.equal(this.newVaultId.toString(), reqEvent.args.newVault.toString());
     assert.equal(this.replaceId.toString(), reqEvent.args.replaceId.toString());
   });
+
+
+  it("Cancel Replace", async function () {
+    await web3.miner.revert(this.snapshotId);
+    //deployHelper.printVault(this.OneBtc, this.vaultId);
+
+    const cancelAmount = 0.5 * 1e8;
+    const collateral = await this.ExchangeRateOracleWrapper.wrappedToCollateral(cancelAmount);
+    const griefingCollateral = (collateral * 5 / 100).toString(); // 5%
+
+    const CancelReq = await this.OneBtc.cancelReplace(this.vaultId, cancelAmount,  { from: this.vaultId });
+    const CancelEvent = CancelReq.logs.filter(
+      (log) => log.event == "WithdrawReplace"
+    )[0];
+
+    assert.equal(CancelEvent.args.withdrawnTokens.toString(), cancelAmount.toString());
+    assert.equal(CancelEvent.args.withdrawnGriefingCollateral.toString(), griefingCollateral.toString());
+    //await deployHelper.printVault(this.OneBtc, this.vaultId);
+
+  });  
 });

contract/bridge/test/helpers/deploy.js

@@ -22,6 +22,14 @@ async function deployOneBTC() {
     return {oneBtc, relayMock, exchangeRateOracleWrapper}
 }
 
+async function printVault(contract, vaultId) {
+    const vaultInfo = await contract.vaults(vaultId);
+    for(var k in vaultInfo) {
+        isNaN(k) && console.log(k, vaultInfo[k].toString())
+    }
+}
+
 module.exports = {
-    deployOneBTC
+    deployOneBTC,
+    printVault
 }