-
Notifications
You must be signed in to change notification settings - Fork 5
ERC20: enable a subnet's supply to be determined by an ERC20 token in the parent. #313
Changes from all commits
94f581e
66e7a05
00b1b67
cd36e50
e626bc4
7ad9dbe
df08205
5039643
fef2161
99c8053
bc291ce
75966e2
cd1c4be
1a8c60e
a5422ab
181ad95
10d954f
5a80b23
7abf037
4079663
156e8f6
5c0476c
a6272c9
633c033
e45ba91
38374ef
e85548d
3d6406e
71eed55
60af071
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,23 +2,29 @@ | |
pragma solidity 0.8.19; | ||
|
||
import {GatewayActorModifiers} from "../lib/LibGatewayActorStorage.sol"; | ||
import {SubnetActorGetterFacet} from "../subnet/SubnetActorGetterFacet.sol"; | ||
import {BURNT_FUNDS_ACTOR} from "../constants/Constants.sol"; | ||
import {CrossMsg} from "../structs/CrossNet.sol"; | ||
import {Status} from "../enums/Status.sol"; | ||
import {FvmAddress} from "../structs/FvmAddress.sol"; | ||
import {SubnetID, Subnet} from "../structs/Subnet.sol"; | ||
import {SubnetID, Subnet, SupplySource} from "../structs/Subnet.sol"; | ||
import {Membership, SupplyKind} from "../structs/Subnet.sol"; | ||
import {AlreadyRegisteredSubnet, CannotReleaseZero, MethodNotAllowed, NotEnoughFunds, NotEnoughFundsToRelease, NotEnoughCollateral, NotEmptySubnetCircSupply, NotRegisteredSubnet, InvalidCrossMsgValue} from "../errors/IPCErrors.sol"; | ||
import {LibGateway} from "../lib/LibGateway.sol"; | ||
import {SubnetIDHelper} from "../lib/SubnetIDHelper.sol"; | ||
import {CrossMsgHelper} from "../lib/CrossMsgHelper.sol"; | ||
import {FilAddress} from "fevmate/utils/FilAddress.sol"; | ||
import {ReentrancyGuard} from "../lib/LibReentrancyGuard.sol"; | ||
import {IERC20} from "openzeppelin-contracts/token/ERC20/IERC20.sol"; | ||
import {Address} from "openzeppelin-contracts/utils/Address.sol"; | ||
import {SupplySourceHelper} from "../lib/SupplySourceHelper.sol"; | ||
|
||
string constant ERR_CHILD_SUBNET_NOT_ALLOWED = "Subnet does not allow child subnets"; | ||
|
||
contract GatewayManagerFacet is GatewayActorModifiers, ReentrancyGuard { | ||
using FilAddress for address payable; | ||
using SubnetIDHelper for SubnetID; | ||
using SupplySourceHelper for SupplySource; | ||
|
||
/// @notice register a subnet in the gateway. It is called by a subnet when it reaches the threshold stake | ||
/// @dev The subnet can optionally pass a genesis circulating supply that would be pre-allocated in the | ||
|
@@ -145,19 +151,53 @@ contract GatewayManagerFacet is GatewayActorModifiers, ReentrancyGuard { | |
// prevent spamming if there's no value to fund. | ||
revert InvalidCrossMsgValue(); | ||
} | ||
// slither-disable-next-line unused-return | ||
(bool registered, ) = LibGateway.getSubnet(subnetId); | ||
if (!registered) { | ||
revert NotRegisteredSubnet(); | ||
} | ||
|
||
// Validate that the supply strategy is native. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
SupplySource memory supplySource = SubnetActorGetterFacet(subnetId.getActor()).supplySource(); | ||
supplySource.expect(SupplyKind.Native); | ||
|
||
CrossMsg memory crossMsg = CrossMsgHelper.createFundMsg({ | ||
subnet: subnetId, | ||
signer: msg.sender, | ||
to: to, | ||
value: msg.value, | ||
fee: 0 // injecting funds into a subnet should is free | ||
fee: 0 // injecting funds into a subnet is free | ||
}); | ||
|
||
// commit top-down message. | ||
LibGateway.commitTopDownMsg(crossMsg); | ||
} | ||
|
||
/// @notice release() burns the received value and releases them from this subnet onto the parent by committing a bottom-up message. | ||
function fundWithToken(SubnetID calldata subnetId, FvmAddress calldata to, uint256 amount) external nonReentrant { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we validate the address There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0x0 has a "special case-ness" in ethereum and i'm not 100% familiar with all of the mechanics to it. Here's some discussion -- https://forum.openzeppelin.com/t/removing-address-0x0-checks-from-openzeppelin-contracts/2222/2 interestingly it seems to regard 0x0 as the "burn address" and wants to separate semantics around transfer and burn. we may want to consider 0x0 as our burn address and welcoming this standard There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comments in that discussion sum it up pretty well. Various standards have coalesced towards treating the 0x0 address as the creator and burn sink of things. I think blocking 0x0 at this level may hinder legitimate user flows down the line. That said, enforcing the restriction now and then loosening it up is more backwards compatible than the other way around... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe worth considering consensus-shipyard/fendermint#409 has introduced 0x0 for the system address, so at least it works as a source. Hopefully the fact that it doesn't have an actual Ethereum account doesn't interfere with using it as a burn address. |
||
// Check that the supply strategy is ERC20. | ||
// There is no need to check whether the subnet exists. If it doesn't exist, the call to getter will revert. | ||
// LibGateway.commitTopDownMsg will also revert if the subnet doesn't exist. | ||
SupplySource memory supplySource = SubnetActorGetterFacet(subnetId.getActor()).supplySource(); | ||
supplySource.expect(SupplyKind.ERC20); | ||
|
||
// Lock the specified amount into custody. | ||
supplySource.lock({value: amount}); | ||
|
||
// Create the top-down message to mint the supply in the subnet. | ||
CrossMsg memory crossMsg = CrossMsgHelper.createFundMsg({ | ||
subnet: subnetId, | ||
signer: msg.sender, | ||
to: to, | ||
value: amount, | ||
fee: 0 // injecting funds into a subnet is free | ||
}); | ||
|
||
// Commit top-down message. | ||
LibGateway.commitTopDownMsg(crossMsg); | ||
} | ||
|
||
/// @notice release() burns the received value locally in subnet and commits a bottom-up message to release the assets in the parent. | ||
/// The local supply of a subnet is always the native coin, so this method doesn't have to deal with tokens. | ||
/// | ||
/// @param to: the address to which to credit funds in the parent subnet. | ||
function release(FvmAddress calldata to) external payable { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,20 +5,22 @@ import {GatewayActorModifiers} from "../lib/LibGatewayActorStorage.sol"; | |
import {BURNT_FUNDS_ACTOR} from "../constants/Constants.sol"; | ||
import {CrossMsg, StorableMsg} from "../structs/CrossNet.sol"; | ||
import {IPCMsgType} from "../enums/IPCMsgType.sol"; | ||
import {SubnetID} from "../structs/Subnet.sol"; | ||
import {SubnetID, SupplyKind} from "../structs/Subnet.sol"; | ||
import {InvalidCrossMsgFromSubnet, InvalidCrossMsgDstSubnet, CannotSendCrossMsgToItself, InvalidCrossMsgValue, MethodNotAllowed} from "../errors/IPCErrors.sol"; | ||
import {SubnetIDHelper} from "../lib/SubnetIDHelper.sol"; | ||
import {LibGateway} from "../lib/LibGateway.sol"; | ||
import {StorableMsgHelper} from "../lib/StorableMsgHelper.sol"; | ||
import {FilAddress} from "fevmate/utils/FilAddress.sol"; | ||
import {SupplySourceHelper} from "../lib/SupplySourceHelper.sol"; | ||
|
||
string constant ERR_GENERAL_CROSS_MSG_DISABLED = "Support for general-purpose cross-net messages is disabled"; | ||
string constant ERR_MULTILEVEL_CROSS_MSG_DISABLED = "Support for general-purpose cross-net messages is disabled"; | ||
string constant ERR_MULTILEVEL_CROSS_MSG_DISABLED = "Support for multi-level cross-net messages is disabled"; | ||
|
||
contract GatewayMessengerFacet is GatewayActorModifiers { | ||
using FilAddress for address payable; | ||
using SubnetIDHelper for SubnetID; | ||
using StorableMsgHelper for StorableMsg; | ||
using SupplySourceHelper for address; | ||
|
||
/** | ||
* @dev sends a general-purpose cross-message from the local subnet to the destination subnet. | ||
|
@@ -98,27 +100,44 @@ contract GatewayMessengerFacet is GatewayActorModifiers { | |
SubnetID memory from = crossMessage.message.from.subnetId; | ||
IPCMsgType applyType = crossMessage.message.applyType(s.networkName); | ||
|
||
// slither-disable-next-line uninitialized-local | ||
bool shouldCommitBottomUp; | ||
// Are we the LCA? (Lowest Common Ancestor) | ||
bool isLCA = to.commonParent(from).equals(s.networkName); | ||
|
||
// Even if multi-level messaging is enabled, we reject the xnet message | ||
// as soon as we learn that one of the networks involved use an ERC20 supply source. | ||
// This will block propagation on the first step, or the last step. | ||
// | ||
// TODO IPC does not implement fault handling yet, so if the message fails | ||
// to propagate, the user won't be able to reclaim funds. That's one of the | ||
// reasons xnet messages are disabled by default. | ||
|
||
bool reject = false; | ||
if (applyType == IPCMsgType.BottomUp) { | ||
shouldCommitBottomUp = !to.commonParent(from).equals(s.networkName); | ||
// We're traversing up, so if we're the first hop, we reject if the subnet was ERC20. | ||
// If we're not the first hop, a child propagated this to us, they made a mistake and | ||
// and we don't have enough info to evaluate. | ||
reject = from.getParentSubnet().equals(s.networkName) && from.getActor().hasSupplyOfKind(SupplyKind.ERC20); | ||
} else if (applyType == IPCMsgType.TopDown) { | ||
// We're traversing down. | ||
// Check the next subnet (which can may be the destination subnet). | ||
reject = to.down(s.networkName).getActor().hasSupplyOfKind(SupplyKind.ERC20); | ||
} | ||
|
||
if (shouldCommitBottomUp) { | ||
LibGateway.commitBottomUpMsg(crossMessage); | ||
|
||
// gas-opt: original check: value > 0 | ||
return (shouldBurn = crossMessage.message.value != 0); | ||
if (reject) { | ||
revert MethodNotAllowed("propagation not suppported for subnets with ERC20 supply"); | ||
} | ||
|
||
if (applyType == IPCMsgType.TopDown) { | ||
// If the directionality is top-down, or if we're inverting the direction | ||
// because we're the LCA, commit a top-down message. | ||
if (applyType == IPCMsgType.TopDown || isLCA) { | ||
++s.appliedTopDownNonce; | ||
LibGateway.commitTopDownMsg(crossMessage); | ||
return (shouldBurn = false); | ||
} | ||
|
||
LibGateway.commitTopDownMsg(crossMessage); | ||
|
||
return (shouldBurn = false); | ||
// Else, commit a bottom up message. | ||
LibGateway.commitBottomUpMsg(crossMessage); | ||
// gas-opt: original check: value > 0 | ||
return (shouldBurn = crossMessage.message.value != 0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This assumes that the message value includes the fee to be paid (which is currently the case, but worth noting in case we change the way it works in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think I changed anything here. Do you want to open an issue to track this? |
||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The husky prettier hook insists on making these changes 🤷