From 3ea02914fa5928ef373609c7b0074713f13607a9 Mon Sep 17 00:00:00 2001
From: Mark Spanbroek <mark@spanbroek.net>
Date: Wed, 26 Feb 2025 15:00:13 +0100
Subject: [PATCH] marketplace: simplify withdrawing by client

- removes RequestCancelled event, which was not great anyway
  because it is not emitted at the moment that the request is
  cancelled
---
 contracts/Marketplace.sol | 34 ++++------------------------------
 test/Marketplace.test.js  | 23 +++++++++--------------
 2 files changed, 13 insertions(+), 44 deletions(-)

diff --git a/contracts/Marketplace.sol b/contracts/Marketplace.sol
index 1ba8124..8bb6f5e 100644
--- a/contracts/Marketplace.sol
+++ b/contracts/Marketplace.sol
@@ -462,38 +462,13 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
     _vault.withdraw(fund, account);
   }
 
-  /**
-     * @notice Withdraws remaining storage request funds back to the client that
-     deposited them.
-   * @dev Request must be cancelled, failed or finished, and the
-     transaction must originate from the depositor address.
-   * @param requestId the id of the request
-   */
+  /// Withdraws remaining storage request funds back to the client that
   function withdrawFunds(RequestId requestId) public requestIsKnown(requestId) {
-    Request storage request = _requests[requestId];
-    RequestContext storage context = _requestContexts[requestId];
-
-    if (request.client != msg.sender) revert Marketplace_InvalidClientAddress();
-
-    RequestState state = requestState(requestId);
-    if (
-      state != RequestState.Cancelled &&
-      state != RequestState.Failed &&
-      state != RequestState.Finished
-    ) {
-      revert Marketplace_InvalidState();
-    }
-
-    context.state = state;
-    if (state == RequestState.Cancelled) {
-      emit RequestCancelled(requestId);
-    }
-
-    _removeFromMyRequests(request.client, requestId);
-
     FundId fund = requestId.asFundId();
-    AccountId account = _vault.clientAccount(request.client);
+    AccountId account = _vault.clientAccount(msg.sender);
     _vault.withdraw(fund, account);
+
+    _removeFromMyRequests(msg.sender, requestId);
   }
 
   function withdrawByValidator(RequestId requestId) public {
@@ -649,5 +624,4 @@ contract Marketplace is SlotReservations, Proofs, StateRetrieval, Endian {
   event RequestFailed(RequestId indexed requestId);
   event SlotFilled(RequestId indexed requestId, uint64 slotIndex);
   event SlotFreed(RequestId indexed requestId, uint64 slotIndex);
-  event RequestCancelled(RequestId indexed requestId);
 }
diff --git a/test/Marketplace.test.js b/test/Marketplace.test.js
index 8962ef8..6434b9b 100644
--- a/test/Marketplace.test.js
+++ b/test/Marketplace.test.js
@@ -754,15 +754,18 @@ describe("Marketplace", function () {
     it("rejects withdraw when request not yet timed out", async function () {
       switchAccount(client)
       await expect(marketplace.withdrawFunds(slot.request)).to.be.revertedWith(
-        "Marketplace_InvalidState"
+        "VaultFundNotUnlocked"
       )
     })
 
-    it("rejects withdraw when wrong account used", async function () {
+    it("withdraws nothing when wrong account used", async function () {
       await waitUntilCancelled(marketplace, request)
-      await expect(marketplace.withdrawFunds(slot.request)).to.be.revertedWith(
-        "Marketplace_InvalidClientAddress"
-      )
+
+      const startBalance = await token.balanceOf(host.address)
+      await marketplace.withdrawFunds(slot.request)
+      const endBalance = await token.balanceOf(host.address)
+
+      expect(endBalance - startBalance).to.equal(0)
     })
 
     it("rejects withdraw when in wrong state", async function () {
@@ -779,7 +782,7 @@ describe("Marketplace", function () {
       await waitUntilCancelled(marketplace, request)
       switchAccount(client)
       await expect(marketplace.withdrawFunds(slot.request)).to.be.revertedWith(
-        "Marketplace_InvalidState"
+        "VaultFundNotUnlocked"
       )
     })
 
@@ -796,14 +799,6 @@ describe("Marketplace", function () {
       expect(endBalance - startBalance).to.equal(0)
     })
 
-    it("emits event once request is cancelled", async function () {
-      await waitUntilCancelled(marketplace, request)
-      switchAccount(client)
-      await expect(marketplace.withdrawFunds(slot.request))
-        .to.emit(marketplace, "RequestCancelled")
-        .withArgs(requestId(request))
-    })
-
     it("withdraw rest of funds to the client for finished requests", async function () {
       await waitUntilStarted(marketplace, request, proof, token)
       await waitUntilFinished(marketplace, requestId(request))