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.

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

chore: addressing issues 3, 9 and 36 #100

Open
wants to merge 10 commits into
base: issue_18_34
Choose a base branch
from
Open
15 changes: 7 additions & 8 deletions contracts/BalanceTrackerBase.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@
uint256 public constant MIN_MECH_BALANCE = 2;

// Mech marketplace address
address public immutable mechMarketplace;

Check warning on line 61 in contracts/BalanceTrackerBase.sol

View workflow job for this annotation

GitHub Actions / build

Immutable variables name are set to be in capitalized SNAKE_CASE
// Drainer address
address public immutable drainer;

Check warning on line 63 in contracts/BalanceTrackerBase.sol

View workflow job for this annotation

GitHub Actions / build

Immutable variables name are set to be in capitalized SNAKE_CASE
// Collected fees
uint256 public collectedFees;
// Reentrancy lock
Expand Down Expand Up @@ -111,11 +111,11 @@
}

/// @dev Adjusts final requester balance accounting for possible delivery rate difference (debit).
/// @param requester Requester address.
/// @param requesterBalance Requester balance.
/// @param rateDiff Delivery rate difference.
/// @return Adjusted balance.
function _adjustFinalBalance(address requester, uint256 rateDiff) internal virtual returns (uint256) {
return mapRequesterBalances[requester] + rateDiff;
function _adjustFinalBalance(uint256 requesterBalance, uint256 rateDiff) internal virtual returns (uint256) {
return requesterBalance + rateDiff;
Comment on lines +117 to +118
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optimize for gas usage, no need to read from storage the second time.

}

/// @dev Drains specified amount.
Expand Down Expand Up @@ -189,7 +189,7 @@
address requester,
uint256 numRequests,
uint256 deliveryRate,
bytes memory paymentData
bytes calldata paymentData
) external virtual payable {
// Reentrancy guard
if (_locked == 2) {
Expand Down Expand Up @@ -246,13 +246,11 @@

// Get total mech and requester delivery rates
uint256 totalMechDeliveryRate;
uint256 totalRequesterDeliveryRate;
uint256 totalRateDiff;
for (uint256 i = 0; i < deliveredRequests.length; ++i) {
// Check if request was delivered
if (deliveredRequests[i]) {
totalMechDeliveryRate += mechDeliveryRates[i];
totalRequesterDeliveryRate += requesterDeliveryRates[i];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not actually needed, it reflects delivery rates of all the requesters in this batch, not relevant.


// Check for delivery rate difference
if (requesterDeliveryRates[i] > mechDeliveryRates[i]) {
Expand All @@ -261,7 +259,8 @@
totalRateDiff += rateDiff;

// Adjust requester balance
mapRequesterBalances[requesters[i]] = _adjustFinalBalance(requesters[i], rateDiff);
uint256 requesterBalance = mapRequesterBalances[requesters[i]];
mapRequesterBalances[requesters[i]] = _adjustFinalBalance(requesterBalance, rateDiff);
}
}
}
Expand Down Expand Up @@ -291,7 +290,7 @@
address mech,
address requester,
uint256[] calldata mechDeliveryRates,
bytes memory paymentData
bytes calldata paymentData
) external virtual {
// Reentrancy guard
if (_locked == 2) {
Expand Down
11 changes: 1 addition & 10 deletions contracts/Karma.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
mapping(address => mapping(address => int256)) public mapRequesterMechKarma;

/// @dev Karma initializer.
function initialize() external{
function initialize() external {
if (owner != address(0)) {
revert AlreadyInitialized();
}
Expand All @@ -68,7 +68,7 @@

// Store the karma implementation address
// solhint-disable-next-line avoid-low-level-calls
assembly {

Check warning on line 71 in contracts/Karma.sol

View workflow job for this annotation

GitHub Actions / build

Avoid to use inline assembly. It is acceptable only in rare cases
sstore(KARMA_PROXY, newImplementation)
}

Expand Down Expand Up @@ -147,13 +147,4 @@

emit RequesterMechKarmaChanged(requester, mech, karmaChange);
}

/// @dev Gets the implementation address.
/// @return implementation Implementation address.
function getImplementation() external view returns (address implementation) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this into the proxy as suggested.

// solhint-disable-next-line avoid-low-level-calls
assembly {
implementation := sload(KARMA_PROXY)
}
}
}
10 changes: 6 additions & 4 deletions contracts/MechMarketplace.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,9 @@
/// @title Mech Marketplace - Marketplace for posting and delivering requests served by mechs
/// @author Aleksandr Kuperman - <[email protected]>
/// @author Andrey Lebedev - <[email protected]>
/// @author Mariapia Moscatiello - <[email protected]>
/// @author Silvere Gangloff - <[email protected]>
contract MechMarketplace is IErrorsMarketplace {

Check warning on line 55 in contracts/MechMarketplace.sol

View workflow job for this annotation

GitHub Actions / build

Contract has 19 states declarations but allowed no more than 15
event CreateMech(address indexed mech, uint256 indexed serviceId, address indexed mechFactory);
event RemoveMech(address indexed mech, uint256 indexed serviceId);
event OwnerUpdated(address indexed owner);
Expand All @@ -63,10 +64,10 @@
bytes32[] requestIds);
event MarketplaceDelivery(address indexed deliveryMech, address[] indexed requesters, uint256 numDeliveries,
bytes32[] requestIds, bool[] deliveredRequests);
event Deliver(address indexed mech, address indexed mechServiceMultisig, bytes32 requestId, bytes data);
event Deliver(address indexed mech, address indexed mechServiceMultisig, bytes32 requestId, uint256 deliveryRate,
bytes data);
event MarketplaceDeliveryWithSignatures(address indexed deliveryMech, address indexed requester,
uint256 numDeliveries, bytes32[] requestIds);
event RequesterHashApproved(address indexed requester, bytes32 hash);

enum RequestStatus {
DoesNotExist,
Expand All @@ -88,11 +89,11 @@
uint256 public constant MAX_FEE_FACTOR = 10_000;

// Original chain Id
uint256 public immutable chainId;

Check warning on line 92 in contracts/MechMarketplace.sol

View workflow job for this annotation

GitHub Actions / build

Immutable variables name are set to be in capitalized SNAKE_CASE
// Mech karma contract address
address public immutable karma;

Check warning on line 94 in contracts/MechMarketplace.sol

View workflow job for this annotation

GitHub Actions / build

Immutable variables name are set to be in capitalized SNAKE_CASE
// Service registry contract address
address public immutable serviceRegistry;

Check warning on line 96 in contracts/MechMarketplace.sol

View workflow job for this annotation

GitHub Actions / build

Immutable variables name are set to be in capitalized SNAKE_CASE

// Domain separator value
bytes32 public domainSeparator;
Expand Down Expand Up @@ -257,7 +258,8 @@
nonce++;

// Symmetrical delivery mech event that in general happens when delivery is called directly through the mech
emit Deliver(msg.sender, mechServiceMultisig, requestIds[i], deliverWithSignatures[i].deliveryData);
emit Deliver(msg.sender, mechServiceMultisig, requestIds[i], deliveryRates[i],
deliverWithSignatures[i].deliveryData);
}

// Adjust requester nonce values
Expand Down Expand Up @@ -725,7 +727,7 @@
/// @return deliveredRequests Corresponding set of successful / failed deliveries.
function deliverMarketplace(
bytes32[] calldata requestIds,
uint256[] memory deliveryRates
uint256[] calldata deliveryRates
) external returns (bool[] memory deliveredRequests) {
// Reentrancy guard
if (_locked == 2) {
Expand Down
29 changes: 17 additions & 12 deletions contracts/OlasMech.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
}

/// @dev A Mech that is operated by the multisig of an Olas service
/// @author Aleksandr Kuperman - <[email protected]>
/// @author Andrey Lebedev - <[email protected]>
/// @author Mariapia Moscatiello - <[email protected]>
abstract contract OlasMech is Mech, IErrorsMech, ImmutableStorage {
event MaxDeliveryRateUpdated(uint256 maxDeliveryRate);
event Deliver(address indexed mech, address indexed mechServiceMultisig, bytes32 requestId, bytes data);
event Deliver(address indexed mech, address indexed mechServiceMultisig, bytes32 requestId, uint256 deliveryRate,
bytes data);
event Request(address indexed mech, bytes32 requestId, bytes data);
event RevokeRequest(bytes32 requestId);
event NumRequestsIncrease(uint256 numRequests);
Expand All @@ -28,7 +32,11 @@
// Olas mech version number
string public constant VERSION = "1.0.0";
// Mech marketplace address
address public immutable mechMarketplace;

Check warning on line 35 in contracts/OlasMech.sol

View workflow job for this annotation

GitHub Actions / build

Immutable variables name are set to be in capitalized SNAKE_CASE
// Service Registry address
address public immutable serviceRegistry;

Check warning on line 37 in contracts/OlasMech.sol

View workflow job for this annotation

GitHub Actions / build

Immutable variables name are set to be in capitalized SNAKE_CASE
// Service Id
uint256 public immutable serviceId;
// Mech payment type
bytes32 public immutable paymentType;

Expand Down Expand Up @@ -92,6 +100,8 @@
setUp(initParams);

mechMarketplace = _mechMarketplace;
serviceRegistry = _serviceRegistry;
serviceId = _serviceId;
maxDeliveryRate = _maxDeliveryRate;
paymentType = _paymentType;
}
Expand Down Expand Up @@ -260,7 +270,7 @@
for (uint256 i = 0; i < numRequests; ++i) {
if (deliveredRequests[i]) {
numDeliveries++;
emit Deliver(address(this), msg.sender, requestIds[i], deliveryDatas[i]);
emit Deliver(address(this), msg.sender, requestIds[i], deliveryRates[i], deliveryDatas[i]);
} else {
emit RevokeRequest(requestIds[i]);
}
Expand All @@ -281,7 +291,7 @@
address requester,
DeliverWithSignature[] calldata deliverWithSignatures,
uint256[] calldata deliveryRates,
bytes memory paymentData
bytes calldata paymentData
) external onlyOperator {
IMechMarketplace(mechMarketplace).deliverMarketplaceWithSignatures(requester, deliverWithSignatures,
deliveryRates, paymentData);
Expand All @@ -298,24 +308,19 @@

/// @dev Gets mech token (service registry) address.
/// @return serviceRegistry Service registry address.
function token() external view returns (address serviceRegistry) {
// Get service registry
serviceRegistry = abi.decode(readImmutable(), (address));
function token() external view returns (address ) {
return serviceRegistry;
}

/// @dev Gets mech token Id (service Id).
/// @return serviceId Service Id.
function tokenId() external view returns (uint256 serviceId) {
// Get service Id
(, serviceId) = abi.decode(readImmutable(), (address, uint256));
function tokenId() external view returns (uint256) {
return serviceId;
}

/// @dev Gets mech operator (service multisig).
/// @return Service multisig address.
function getOperator() public view returns (address) {
// Get service registry and service Id
(address serviceRegistry, uint256 serviceId) = abi.decode(readImmutable(), (address, uint256));

(, address multisig, , , , , IServiceRegistry.ServiceState state) =
IServiceRegistry(serviceRegistry).mapServices(serviceId);

Expand Down
2 changes: 1 addition & 1 deletion contracts/interfaces/IBalanceTracker.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface IBalanceTracker {
/// @param mechDeliveryRates Corresponding set of actual charged delivery rates for each request.
/// @param requesterDeliveryRates Corresponding set of requester agreed delivery rates for each request.
function finalizeDeliveryRates(address mech, address[] calldata requesters, bool[] calldata deliveredRequests,
uint256[] memory mechDeliveryRates, uint256[] calldata requesterDeliveryRates) external;
uint256[] calldata mechDeliveryRates, uint256[] calldata requesterDeliveryRates) external;

/// @dev Adjusts mech and requester balances for direct batch request processing.
/// @param mech Mech address.
Expand Down
13 changes: 0 additions & 13 deletions contracts/interfaces/IErrorsMarketplace.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ interface IErrorsMarketplace {
/// @param amount Value amount.
error NoDepositAllowed(uint256 amount);

/// @dev Request Id not found.
/// @param requestId Request Id.
error RequestIdNotFound(bytes32 requestId);

/// @dev Value overflow.
/// @param provided Overflow value.
/// @param max Maximum possible value.
Expand All @@ -46,11 +42,6 @@ interface IErrorsMarketplace {
/// @param account Account address.
error UnauthorizedAccount(address account);

/// @dev Specified service Id is not staked.
/// @param stakingInstance Staking contract instance.
/// @param serviceId Service Id.
error ServiceNotStaked(address stakingInstance, uint256 serviceId);

/// @dev Wrong state of a service.
/// @param state Service state.
/// @param serviceId Service Id.
Expand All @@ -66,10 +57,6 @@ interface IErrorsMarketplace {
/// @param requestId Request Id.
error AlreadyRequested(bytes32 requestId);

/// @dev The request is already delivered.
/// @param requestId Request Id.
error AlreadyDelivered(bytes32 requestId);

/// @dev Wrong payment type.
/// @param paymentType Payment type.
error WrongPaymentType(bytes32 paymentType);
Expand Down
4 changes: 0 additions & 4 deletions contracts/interfaces/IErrorsMech.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ interface IErrorsMech {
/// @param marketplace Required marketplace address.
error MarketplaceOnly(address sender, address marketplace);

/// @dev Request Id not found.
/// @param requestId Request Id.
error RequestIdNotFound(bytes32 requestId);

/// @dev Value overflow.
/// @param provided Overflow value.
/// @param max Maximum possible value.
Expand Down
6 changes: 3 additions & 3 deletions contracts/interfaces/IMechMarketplace.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ interface IMechMarketplace {
/// @param requestIds Set of request ids.
/// @param mechDeliveryRates Corresponding set of actual charged delivery rates for each request.
/// @return deliveredRequests Corresponding set of successful / failed deliveries.
function deliverMarketplace(bytes32[] calldata requestIds, uint256[] memory mechDeliveryRates)
external returns (bool[] calldata deliveredRequests);
function deliverMarketplace(bytes32[] calldata requestIds, uint256[] calldata mechDeliveryRates)
external returns (bool[] memory deliveredRequests);

/// @dev Delivers signed requests.
/// @param requester Requester address.
Expand All @@ -20,5 +20,5 @@ interface IMechMarketplace {
/// @param paymentData Additional payment-related request data, if applicable.
function deliverMarketplaceWithSignatures(
address requester, DeliverWithSignature[] calldata deliverWithSignatures, uint256[] calldata deliveryRates,
bytes memory paymentData) external;
bytes calldata paymentData) external;
}
4 changes: 2 additions & 2 deletions contracts/mechs/native/BalanceTrackerFixedPriceNative.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.28;

import {BalanceTrackerBase, ZeroAddress, InsufficientBalance, TransferFailed} from "../../BalanceTrackerBase.sol";
import {BalanceTrackerBase, ZeroAddress, TransferFailed} from "../../BalanceTrackerBase.sol";
import {IMech} from "../../interfaces/IMech.sol";

interface IToken {
Expand Down Expand Up @@ -76,7 +76,7 @@ contract BalanceTrackerFixedPriceNative is BalanceTrackerBase {
revert TransferFailed(address(0), address(this), account, amount);
}

emit Withdraw(msg.sender, address(0), amount);
emit Withdraw(account, address(0), amount);
}

/// @dev Wraps native token.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.28;

import {BalanceTrackerFixedPriceNative, ZeroAddress, InsufficientBalance} from "../native/BalanceTrackerFixedPriceNative.sol";
import {ZeroValue, ReentrancyGuard} from "../../BalanceTrackerBase.sol";
import {BalanceTrackerFixedPriceNative, ZeroAddress} from "../native/BalanceTrackerFixedPriceNative.sol";
import {ZeroValue, InsufficientBalance} from "../../BalanceTrackerBase.sol";

interface IERC1155 {
/// @dev Gets the amount of tokens owned by a specified account.
Expand Down
2 changes: 1 addition & 1 deletion contracts/mechs/token/BalanceTrackerFixedPriceToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ contract BalanceTrackerFixedPriceToken is BalanceTrackerBase {
// Transfer tokens
IToken(token).transfer(account, amount);

emit Withdraw(msg.sender, token, amount);
emit Withdraw(account, token, amount);
}

/// @dev Deposits token funds for requester.
Expand Down
9 changes: 9 additions & 0 deletions contracts/proxies/KarmaProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,13 @@ contract KarmaProxy {
return(0, returndatasize())
}
}

/// @dev Gets the implementation address.
/// @return implementation Implementation address.
function getImplementation() external view returns (address implementation) {
// solhint-disable-next-line avoid-low-level-calls
assembly {
implementation := sload(KARMA_PROXY)
}
}
}
Binary file modified docs/MechMarketplaceDescriptionAndContractsOverviewRepo.pdf
Binary file not shown.
6 changes: 6 additions & 0 deletions test/MechFixedPriceNative.js
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,12 @@ describe("MechFixedPriceNative", function () {
// Deliver requests
await priorityMech.deliverMarketplaceWithSignatures(mockOperatorContract.address, deliverWithSignatures,
deliveryRates, "0x");

// Try to adjustMechRequesterBalances not by marketplace
await expect(
balanceTrackerFixedPriceNative.adjustMechRequesterBalances(priorityMech.address, deployer.address,
[maxDeliveryRate], data)
).to.be.revertedWithCustomError(balanceTrackerFixedPriceNative, "MarketplaceOnly");
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/MechNvmSubscriptionNative.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const { expect } = require("chai");
const { config, ethers } = require("hardhat");

describe.only("MechNvmSubscriptionNative", function () {
describe("MechNvmSubscriptionNative", function () {
let priorityMechAddress;
let priorityMech;
let serviceRegistry;
Expand Down
Loading