From cb283c8ac93e527ae1e5f8dd51f9553102df8b51 Mon Sep 17 00:00:00 2001 From: Aleksandr Kuperman Date: Thu, 26 Dec 2024 14:39:04 +0000 Subject: [PATCH] fix: more bugs --- contracts/BalanceTrackerFixedPriceBase.sol | 43 ++++++++++++++----- contracts/BalanceTrackerFixedPriceNative.sol | 8 ++-- contracts/BalanceTrackerFixedPriceToken.sol | 8 ++-- contracts/MechMarketplace.sol | 8 ++-- .../BalanceTrackerNvmSubscription.sol | 3 +- test/MechFixedPriceNative.js | 27 ++++++++++++ 6 files changed, 74 insertions(+), 23 deletions(-) diff --git a/contracts/BalanceTrackerFixedPriceBase.sol b/contracts/BalanceTrackerFixedPriceBase.sol index 600e261..b64f4e8 100644 --- a/contracts/BalanceTrackerFixedPriceBase.sol +++ b/contracts/BalanceTrackerFixedPriceBase.sol @@ -52,8 +52,8 @@ abstract contract BalanceTrackerFixedPriceBase { event Withdraw(address indexed account, address indexed token, uint256 amount); event Drained(address indexed token, uint256 collectedFees); - // Max marketplace fee - uint256 public constant MAX_FEE = 10_000; + // Max marketplace fee factor (100%) + uint256 public constant MAX_FEE_FACTOR = 10_000; // Mech marketplace address address public immutable mechMarketplace; @@ -189,10 +189,33 @@ abstract contract BalanceTrackerFixedPriceBase { _locked = 1; } - function _withdraw(uint256 balance) internal virtual; + function _withdraw(address mech, uint256 balance) internal virtual; - /// @dev Processes mech payment by withdrawing funds. - function processPayment() external returns (uint256 mechPayment, uint256 marketplaceFee) { + /// @dev Processes mech payment by mech service multisig. + /// @param mech Mech address. + /// @return Mech payment. + /// @return Marketplace fee. + function processPaymentByMultisig(address mech) external returns (uint256, uint256) { + // Check for mech service multisig address + if (!IMech(mech).isOperator(msg.sender)) { + revert UnauthorizedAccount(msg.sender); + } + + return _processPayment(mech); + } + + /// @dev Processes mech payment. + /// @return Mech payment. + /// @return Marketplace fee. + function processPayment() external returns (uint256, uint256) { + return _processPayment(msg.sender); + } + + /// @dev Process mech payment. + /// @param mech Mech address. + /// @return mechPayment Mech payment. + /// @return marketplaceFee Marketplace fee. + function _processPayment(address mech) internal returns (uint256 mechPayment, uint256 marketplaceFee) { // Reentrancy guard if (_locked > 1) { revert ReentrancyGuard(); @@ -200,7 +223,7 @@ abstract contract BalanceTrackerFixedPriceBase { _locked = 2; // Get mech balance - uint256 balance = mapMechBalances[msg.sender]; + uint256 balance = mapMechBalances[mech]; if (balance == 0) { revert ZeroValue(); } @@ -211,7 +234,7 @@ abstract contract BalanceTrackerFixedPriceBase { // If requested balance is too small, charge the minimal fee // ceil(a, b) = (a + b - 1) / b // This formula will always get at least a fee of 1 - marketplaceFee = (balance * fee + (MAX_FEE - 1)) / MAX_FEE; + marketplaceFee = (balance * fee + (MAX_FEE_FACTOR - 1)) / MAX_FEE_FACTOR; // Calculate mech payment mechPayment = balance - marketplaceFee; @@ -225,10 +248,10 @@ abstract contract BalanceTrackerFixedPriceBase { collectedFees += marketplaceFee; // Clear balances - mapMechBalances[msg.sender] = 0; + mapMechBalances[mech] = 0; // Process withdraw - _withdraw(balance); + _withdraw(mech, mechPayment); _locked = 1; } @@ -251,7 +274,7 @@ abstract contract BalanceTrackerFixedPriceBase { mapRequesterBalances[msg.sender] = 0; // Process withdraw - _withdraw(balance); + _withdraw(msg.sender, balance); _locked = 1; } diff --git a/contracts/BalanceTrackerFixedPriceNative.sol b/contracts/BalanceTrackerFixedPriceNative.sol index 067f1ee..47be330 100644 --- a/contracts/BalanceTrackerFixedPriceNative.sol +++ b/contracts/BalanceTrackerFixedPriceNative.sol @@ -61,16 +61,16 @@ contract BalanceTrackerFixedPriceNative is BalanceTrackerFixedPriceBase { emit Drained(wrappedNativeToken, amount); } - function _withdraw(uint256 balance) internal virtual override { + function _withdraw(address account, uint256 amount) internal virtual override { // solhint-disable-next-line avoid-low-level-calls - (bool success, ) = msg.sender.call{value: balance}(""); + (bool success, ) = account.call{value: amount}(""); // Check transfer if (!success) { - revert TransferFailed(address(0), address(this), msg.sender, balance); + revert TransferFailed(address(0), address(this), account, amount); } - emit Withdraw(msg.sender, address(0), balance); + emit Withdraw(msg.sender, address(0), amount); } // Deposits funds for requester. diff --git a/contracts/BalanceTrackerFixedPriceToken.sol b/contracts/BalanceTrackerFixedPriceToken.sol index 12239f5..f9536c8 100644 --- a/contracts/BalanceTrackerFixedPriceToken.sol +++ b/contracts/BalanceTrackerFixedPriceToken.sol @@ -76,15 +76,15 @@ contract BalanceTrackerFixedPriceToken is BalanceTrackerFixedPriceBase { emit Drained(olas, amount); } - function _withdraw(uint256 balance) internal virtual override { - bool success = IToken(olas).transfer(msg.sender, balance); + function _withdraw(address account, uint256 amount) internal virtual override { + bool success = IToken(olas).transfer(account, amount); // Check transfer if (!success) { - revert TransferFailed(olas, address(this), msg.sender, balance); + revert TransferFailed(olas, address(this), account, amount); } - emit Withdraw(msg.sender, olas, balance); + emit Withdraw(msg.sender, olas, amount); } // Deposits token funds for requester. diff --git a/contracts/MechMarketplace.sol b/contracts/MechMarketplace.sol index 6865ca0..1f8580d 100644 --- a/contracts/MechMarketplace.sol +++ b/contracts/MechMarketplace.sol @@ -61,8 +61,8 @@ contract MechMarketplace is IErrorsMarketplace { // Domain separator type hash bytes32 public constant DOMAIN_SEPARATOR_TYPE_HASH = keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"); - // Max marketplace fee - uint256 public constant MAX_FEE = 10_000; + // Max marketplace fee factor (100%) + uint256 public constant MAX_FEE_FACTOR = 10_000; // Original domain separator value bytes32 public immutable domainSeparator; @@ -160,8 +160,8 @@ contract MechMarketplace is IErrorsMarketplace { } // Check for fee value - if (newFee > MAX_FEE) { - revert Overflow(newFee, MAX_FEE); + if (newFee > MAX_FEE_FACTOR) { + revert Overflow(newFee, MAX_FEE_FACTOR); } // Check for sanity values diff --git a/contracts/integrations/nevermined/BalanceTrackerNvmSubscription.sol b/contracts/integrations/nevermined/BalanceTrackerNvmSubscription.sol index 4c39c1d..a85946a 100644 --- a/contracts/integrations/nevermined/BalanceTrackerNvmSubscription.sol +++ b/contracts/integrations/nevermined/BalanceTrackerNvmSubscription.sol @@ -172,7 +172,8 @@ contract BalanceTrackerNvmSubscription { _locked = 1; } - /// @dev Processes payment. + /// @dev Processes requester credits. + /// @param requester Requester address. function processPayment(address requester) external { // Reentrancy guard if (_locked > 1) { diff --git a/test/MechFixedPriceNative.js b/test/MechFixedPriceNative.js index 45f5fe7..9130c40 100644 --- a/test/MechFixedPriceNative.js +++ b/test/MechFixedPriceNative.js @@ -297,6 +297,33 @@ describe("MechFixedPriceNative", function () { // Check requester mech karma mechKarma = await karma.mapRequesterMechKarma(deployer.address, priorityMech.address); expect(mechKarma).to.equal(1); + + // Check priority mech balance now + let mechBalance = await balanceTrackerFixedPriceNative.mapMechBalances(priorityMech.address); + expect(mechBalance).to.equal(maxDeliveryRate); + + const balanceBefore = await ethers.provider.getBalance(priorityMech.address); + // Process payment for mech + await balanceTrackerFixedPriceNative.processPaymentByMultisig(priorityMech.address); + const balanceAfter = await ethers.provider.getBalance(priorityMech.address); + + // Check charged fee + const collectedFees = await balanceTrackerFixedPriceNative.collectedFees(); + // Since the delivery rate is smaller than MAX_FEE_FACTOR, the minimal fee was charged + expect(collectedFees).to.equal(1); + + // Check mech payout: payment - fee + const balanceDiff = balanceAfter.sub(balanceBefore); + expect(balanceDiff).to.equal(maxDeliveryRate - 1); + + // Check requester leftover balance + let requesterBalance = await balanceTrackerFixedPriceNative.mapRequesterBalances(deployer.address); + expect(requesterBalance).to.equal(maxDeliveryRate - 1); + + // Withdraw requester balances + await balanceTrackerFixedPriceNative.withdraw(); + requesterBalance = await balanceTrackerFixedPriceNative.mapRequesterBalances(deployer.address); + expect(requesterBalance).to.equal(0); }); it("Delivering a request by a different mech", async function () {