feat(node): address review comments against #1130

contracts/contracts/errors/IPCErrors.sol

@@ -80,8 +80,6 @@ error InvalidFederationPayload();
 error DuplicatedGenesisValidator();
 error NotEnoughGenesisValidators();
 error ValidatorPowerChangeDenied();
-error IncreaseAllowanceFailed();
-error Unreachable();
 
 enum InvalidXnetMessageReason {
     Sender,

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();
-            }
-        }
-    }
-}
+}

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: amount}(amount);
                 } else {
-                    revert Unreachable();
+                    revert("Unknown staking operation");
                 }
             }
 

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);
     }