diff --git a/contracts/contracts/errors/IPCErrors.sol b/contracts/contracts/errors/IPCErrors.sol index 7bdb537c58..7f838e1a42 100644 --- a/contracts/contracts/errors/IPCErrors.sol +++ b/contracts/contracts/errors/IPCErrors.sol @@ -80,8 +80,6 @@ error InvalidFederationPayload(); error DuplicatedGenesisValidator(); error NotEnoughGenesisValidators(); error ValidatorPowerChangeDenied(); -error IncreaseAllowanceFailed(); -error Unreachable(); enum InvalidXnetMessageReason { Sender, diff --git a/contracts/contracts/lib/AssetHelper.sol b/contracts/contracts/lib/AssetHelper.sol index f81829a018..07e52ff323 100644 --- a/contracts/contracts/lib/AssetHelper.sol +++ b/contracts/contracts/lib/AssetHelper.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.23; -import {NotEnoughBalance, IncreaseAllowanceFailed} from "../errors/IPCErrors.sol"; +import {NotEnoughBalance} from "../errors/IPCErrors.sol"; import {Asset, AssetKind} from "../structs/Subnet.sol"; import {EMPTY_BYTES} from "../constants/Constants.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; @@ -205,21 +205,22 @@ library AssetHelper { } } - function native() internal pure returns (Asset memory) { - return Asset({kind: AssetKind.Native, tokenAddress: address(0)}); + // @notice Makes the asset available for spending by the given spender, without actually sending it. + // @return msgValue The amount of msg.value that needs to be sent along with the subsequent call that will _actually_ spend that asset. + // Will be 0 if the asset is a token, since no native coins are to be sent. + function makeAvailable(Asset memory self, address spender, uint256 amount) internal returns (uint256 msgValue) { + if (self.kind == AssetKind.Native) { + msgValue = amount; + } else if (self.kind == AssetKind.ERC20) { + IERC20 token = IERC20(self.tokenAddress); + token.safeIncreaseAllowance(spender, amount); + msgValue = 0; + } + return msgValue; } - function isNative(Asset memory self) internal pure returns(bool) { - return self.kind == AssetKind.Native; + function native() internal pure returns (Asset memory) { + return Asset({kind: AssetKind.Native, tokenAddress: address(0)}); } - function increaseAllowance(Asset memory self, address spender, uint256 amount) internal { - if (self.kind == AssetKind.ERC20) { - IERC20 token = IERC20(self.tokenAddress); - uint256 allowance = token.allowance(address(this), spender); - if (!token.approve(spender, allowance + amount)) { - revert IncreaseAllowanceFailed(); - } - } - } -} +} \ No newline at end of file diff --git a/contracts/contracts/lib/LibStaking.sol b/contracts/contracts/lib/LibStaking.sol index 5d518146c2..cfb1f7a796 100644 --- a/contracts/contracts/lib/LibStaking.sol +++ b/contracts/contracts/lib/LibStaking.sol @@ -8,7 +8,7 @@ import {LibMinPQ, MinPQ} from "./priority/LibMinPQ.sol"; import {LibStakingChangeLog} from "./LibStakingChangeLog.sol"; import {AssetHelper} from "./AssetHelper.sol"; import {PermissionMode, StakingReleaseQueue, StakingChangeLog, StakingChange, StakingChangeRequest, StakingOperation, StakingRelease, ValidatorSet, AddressStakingReleases, ParentValidatorsTracker, Validator, Asset} from "../structs/Subnet.sol"; -import {WithdrawExceedingCollateral, NotValidator, CannotConfirmFutureChanges, NoCollateralToWithdraw, AddressShouldBeValidator, InvalidConfigurationNumber, Unreachable} from "../errors/IPCErrors.sol"; +import {WithdrawExceedingCollateral, NotValidator, CannotConfirmFutureChanges, NoCollateralToWithdraw, AddressShouldBeValidator, InvalidConfigurationNumber} from "../errors/IPCErrors.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; library LibAddressStakingReleases { @@ -583,15 +583,10 @@ library LibStaking { IGateway(gateway).releaseStake(amount); } else if (change.op == StakingOperation.Deposit) { s.validatorSet.confirmDeposit(validator, amount); - - if (s.collateralSource.isNative()) { - IGateway(gateway).addStake{value: amount}(amount); - } else { - s.collateralSource.increaseAllowance(gateway, amount); - IGateway(gateway).addStake(amount); - } + uint256 msgValue = s.collateralSource.makeAvailable(gateway, amount); + IGateway(gateway).addStake{value: msgValue}(amount); } else { - revert Unreachable(); + revert("Unknown staking operation"); } } diff --git a/contracts/contracts/lib/LibSubnetActor.sol b/contracts/contracts/lib/LibSubnetActor.sol index be90c2fa1c..14731f7685 100644 --- a/contracts/contracts/lib/LibSubnetActor.sol +++ b/contracts/contracts/lib/LibSubnetActor.sol @@ -161,26 +161,9 @@ library LibSubnetActor { uint256 genesisCircSupply = s.genesisCircSupply; - bool supplySourceNative = s.supplySource.isNative(); - bool collateralSourceNative = s.collateralSource.isNative(); - - // this method is unnecessarily handling different cases because subnet actor needs - // to "register" in gateway and different token types needs to be attached or approved. - // TODO: it's known that having gateway holding all subnets' funds is insecure, this - // TODO: can be removed once contract redesign is in place. uint256 msgValue = 0; - - if (!supplySourceNative) { - s.supplySource.increaseAllowance(s.ipcGatewayAddr, genesisCircSupply); - } else { - msgValue += genesisCircSupply; - } - - if (!collateralSourceNative) { - s.collateralSource.increaseAllowance(s.ipcGatewayAddr, collateral); - } else { - msgValue += collateral; - } + msgValue += s.supplySource.makeAvailable(s.ipcGatewayAddr, genesisCircSupply); + msgValue += s.collateralSource.makeAvailable(s.ipcGatewayAddr, collateral); IGateway(s.ipcGatewayAddr).register{value: msgValue}(genesisCircSupply, collateral); }