Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions contracts/contracts/errors/IPCErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ error OldConfigurationNumber();
error PQDoesNotContainAddress();
error PQEmpty();
error ParentFinalityAlreadyCommitted();
error PostboxNotExist();
error SignatureReplay();
error SubnetAlreadyKilled();
error SubnetNotActive();
Expand Down Expand Up @@ -91,7 +90,8 @@ enum InvalidXnetMessageReason {
Kind,
ReflexiveSend,
NoRoute,
IncompatibleSupplySource
IncompatibleSupplySource,
NonDirect
}

string constant ERR_PERMISSIONED_AND_BOOTSTRAPPED = "Method not allowed if permissioned is enabled and subnet bootstrapped";
Expand Down
10 changes: 0 additions & 10 deletions contracts/contracts/gateway/GatewayGetterFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,6 @@ contract GatewayGetterFacet {
return s.appliedTopDownNonce;
}

/// @notice Returns the storable message and its wrapped status from the postbox by a given identifier.
/// @param id The unique identifier of the message in the postbox.
function postbox(bytes32 id) external view returns (IpcEnvelope memory storableMsg) {
return (s.postbox[id]);
}

function postboxMsgs() external view returns (bytes32[] memory) {
return (s.postboxKeys.values());
}

/// @notice Returns the majority percentage required for certain consensus or decision-making processes.
function majorityPercentage() external view returns (uint64) {
return s.majorityPercentage;
Expand Down
7 changes: 0 additions & 7 deletions contracts/contracts/gateway/GatewayMessengerFacet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,4 @@ contract GatewayMessengerFacet is GatewayActorModifiers {
// which passes the struct by reference.
return committed;
}

/**
* @dev Propagates all the populated cross-net messages from the postbox.
*/
function propagateAll() external payable {
LibGateway.propagateAllPostboxMessages();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,5 @@ contract XnetMessagingFacet is GatewayActorModifiers {
/// @param crossMsgs The array of cross-network messages to be applied.
function applyCrossMessages(IpcEnvelope[] calldata crossMsgs) external systemActorOnly {
LibGateway.applyTopDownMessages(s.networkName.getParentSubnet(), crossMsgs);
LibGateway.propagateAllPostboxMessages();
}
}
3 changes: 0 additions & 3 deletions contracts/contracts/interfaces/IGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ interface IGateway {
IpcEnvelope calldata envelope
) external payable returns (IpcEnvelope memory committed);

/// @notice Propagates all the stored messages to destination subnet
function propagateAll() external payable;

/// @notice commit the ipc parent finality into storage
function commitParentFinality(ParentFinality calldata finality) external;
}
172 changes: 10 additions & 162 deletions contracts/contracts/lib/LibGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ library LibGateway {
event QueuedBottomUpMessage(bytes32 indexed id);
/// @dev event emitted when there is a new bottom-up message batch to be signed.
event NewBottomUpMsgBatch(uint256 indexed epoch);
/// @dev event emmitted when a message is stored in the postbox - to be propagated further.
event MessageStoredInPostbox(bytes32 indexed id);
/// @dev event emmitted when a message is propagated further from the postbox.
event MessagePropagatedFromPostbox(bytes32 id);

/// @notice returns the bottom-up batch
function getBottomUpMsgBatch(uint256 epoch) internal view returns (bool exists, BottomUpMsgBatch storage batch) {
Expand Down Expand Up @@ -288,7 +284,7 @@ library LibGateway {
}
}

/// @notice executes a cross message if its destination is the current network, otherwise adds it to the postbox to be propagated further
/// @notice executes a cross message, assuming its destination is the current network.
/// This function assumes that the relevant funds have been already minted or burnt
/// when the top-down or bottom-up messages have been queued for execution.
/// This function is not expected to revert. If a controlled failure happens, a new
Expand Down Expand Up @@ -355,28 +351,8 @@ library LibGateway {
supplySource = AssetHelper.native();
}

// If the crossnet destination is NOT the current network (network where the gateway is running),
// we add it to the postbox for further propagation.
// Even if we send for propagation, the execution of every message
// should increase the appliedNonce to allow the execution of the next message
// of the batch (this is way we have this after the nonce logic).
if (!crossMsg.to.subnetId.equals(s.networkName)) {
(bool valid, InvalidXnetMessageReason reason) = validateCrossMessage(crossMsg);
if (!valid) {
sendReceipt(
crossMsg,
OutcomeType.SystemErr,
abi.encodeWithSelector(InvalidXnetMessage.selector, reason)
);
return;
}

bytes32 cid = crossMsg.toHash();
s.postboxKeys.add(cid);
s.postbox[cid] = crossMsg;

emit MessageStoredInPostbox({id: crossMsg.toTracingId()});
return;
revert("Only direct messaging supported");
}

// execute the message and get the receipt.
Expand Down Expand Up @@ -440,11 +416,8 @@ library LibGateway {

SubnetID memory to = crossMessage.to.subnetId;
IPCMsgType applyType = crossMessage.applyType(s.networkName);
bool isLCA = to.commonParent(crossMessage.from.subnetId).equals(s.networkName);

// 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) {
if (applyType == IPCMsgType.TopDown) {
(, SubnetID memory subnetId) = to.down(s.networkName);
(, Subnet storage subnet) = getSubnet(subnetId);
LibGateway.commitTopDownMsg(subnet, crossMessage);
Expand All @@ -471,52 +444,6 @@ library LibGateway {
}
}

/// Checks if the incoming and outgoing subnet supply sources can be mapped.
/// Caller should make sure the incoming/outgoing subnets and current subnet are immediate parent/child subnets.
function checkSubnetsSupplyCompatible(
bool isLCA,
IPCMsgType applyType,
SubnetID memory incoming,
SubnetID memory outgoing,
SubnetID memory current
) internal view returns(bool) {
if (isLCA) {
// now, it's pivoting @ LCA (i.e. upwards => downwards)
// if incoming bottom up subnet and outgoing target subnet have the same
// asset, we will allow it. This is because if they are using the
// same asset, then the asset can be mapped in both subnets.

(, SubnetID memory incDown) = incoming.down(current);
(, SubnetID memory outDown) = outgoing.down(current);

Asset memory incAsset = ISubnetActor(incDown.getActor()).supplySource();
Asset memory outAsset = ISubnetActor(outDown.getActor()).supplySource();

return incAsset.equals(outAsset);
}

if (applyType == IPCMsgType.BottomUp) {
// The child subnet has supply source native, this is the same as
// the current subnet's native source, the mapping makes sense, propagate up.
(, SubnetID memory incDown) = incoming.down(current);
return incDown.getActor().hasSupplyOfKind(AssetKind.Native);
}

// Topdown handling

// The incoming subnet's supply source will be mapped to native coin in the
// next child subnet. If the down subnet has native, then the mapping makes
// sense.
(, SubnetID memory down) = outgoing.down(current);
return down.getActor().hasSupplyOfKind(AssetKind.Native);
}

/// @notice Validates a cross message before committing it.
function validateCrossMessage(IpcEnvelope memory envelope) internal view returns (bool, InvalidXnetMessageReason) {
(bool valid, InvalidXnetMessageReason reason, ) = checkCrossMessage(envelope);
return (valid, reason);
}

/// @notice Validates a cross message and returns the applyType if the message is valid
function checkCrossMessage(IpcEnvelope memory envelope) internal view returns (bool valid, InvalidXnetMessageReason reason, IPCMsgType applyType) {
SubnetID memory toSubnetId = envelope.to.subnetId;
Expand All @@ -526,101 +453,22 @@ library LibGateway {

GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();
SubnetID memory currentNetwork = s.networkName;
applyType = envelope.applyType(currentNetwork);

// We cannot send a cross message to the same subnet.
if (toSubnetId.equals(currentNetwork)) {
return (false, InvalidXnetMessageReason.ReflexiveSend, applyType);
}

// Lowest common ancestor subnet
bool isLCA = toSubnetId.commonParent(envelope.from.subnetId).equals(currentNetwork);
applyType = envelope.applyType(currentNetwork);

// If the directionality is top-down, or if we're inverting the direction
// else we need to check if the common parent exists.
if (applyType == IPCMsgType.TopDown || isLCA) {
(bool foundChildSubnetId, SubnetID memory childSubnetId) = toSubnetId.down(currentNetwork);
if (!foundChildSubnetId) {
return (false, InvalidXnetMessageReason.DstSubnet, applyType);
}

(bool foundSubnet,) = LibGateway.getSubnet(childSubnetId);
if (!foundSubnet) {
return (false, InvalidXnetMessageReason.DstSubnet, applyType);
}
} else {
SubnetID memory commonParent = toSubnetId.commonParent(currentNetwork);
if (commonParent.isEmpty()) {
return (false, InvalidXnetMessageReason.NoRoute, applyType);
}
}

// starting/ending subnet, no need check supply sources
if (envelope.from.subnetId.equals(currentNetwork) || envelope.to.subnetId.equals(currentNetwork)) {
return (true, reason, applyType);
}

bool supplySourcesCompatible = checkSubnetsSupplyCompatible({
isLCA: isLCA,
applyType: applyType,
incoming: envelope.from.subnetId,
outgoing: envelope.to.subnetId,
current: currentNetwork
});
// Allow only direct parent-child relationships, by checking route length difference.
bool isDirect = (applyType == IPCMsgType.TopDown)
? toSubnetId.route.length == envelope.from.subnetId.route.length + 1
: envelope.from.subnetId.route.length == toSubnetId.route.length + 1;

if (!supplySourcesCompatible) {
return (false, InvalidXnetMessageReason.IncompatibleSupplySource, applyType);
if (!isDirect) {
return (false, InvalidXnetMessageReason.NonDirect, applyType);
Copy link

Choose a reason for hiding this comment

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

Direct message validation allows cross-branch messages

Medium Severity

The isDirect validation in checkCrossMessage only checks that route lengths differ by exactly 1, but doesn't verify the subnets are actually in a parent-child relationship. A message from /root/A/B to /root/X (different branch, not the actual parent /root/A) would pass validation because both have route length difference of 1. For BottomUp messages, this would allow the message to be committed to the checkpoint. When the parent /root/A processes it, applyMsg sees destination /root/X doesn't equal current network and triggers a hard revert("Only direct messaging supported"), potentially failing the entire message batch execution.

Fix in Cursor Fix in Web

}

return (true, reason, applyType);
}

/**
* @dev Propagates all the populated cross-net messages from the postbox.
*/
function propagateAllPostboxMessages() internal {
GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();

uint256 keysLength = s.postboxKeys.length();

bytes32[] memory values = s.postboxKeys.values();

for (uint256 i = 0; i < keysLength; ) {
bytes32 msgCid = values[i];
LibGateway.propagatePostboxMessage(msgCid);

unchecked {
++i;
}
}
}

/**
* @dev Propagates the populated cross-net message for the given `msgCid`.
* @param msgCid - the cid of the cross-net message
*/
function propagatePostboxMessage(bytes32 msgCid) internal {
GatewayActorStorage storage s = LibGatewayActorStorage.appStorage();
IpcEnvelope storage crossMsg = s.postbox[msgCid];

if (crossMsg.isEmpty()) {
revert("Message not found in postbox");
}

bool shouldBurn = LibGateway.commitValidatedCrossMessage(crossMsg);

// Cache value before deletion to avoid re-entrancy
uint256 v = crossMsg.value;
bytes32 deterministicId = crossMsg.toTracingId();

// Remove the message to prevent re-entrancy and clean up state
delete s.postbox[msgCid];
s.postboxKeys.remove(msgCid);

// Execute side effects
LibGateway.crossMsgSideEffects({v: v, shouldBurn: shouldBurn});

emit MessagePropagatedFromPostbox({id: deterministicId});
}

}
6 changes: 0 additions & 6 deletions contracts/contracts/lib/LibGatewayActorStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,6 @@ struct GatewayActorStorage {
mapping(bytes32 => Subnet) subnets;
/// @notice The parent finalities. Key is the block number, value is the finality struct.
mapping(uint256 => ParentFinality) finalitiesMap;
/// @notice Postbox keeps track of all the cross-net messages triggered by
/// an actor that need to be propagated further through the hierarchy.
/// cross-net message id => CrossMsg
mapping(bytes32 => IpcEnvelope) postbox;
/// @notice Keys of the envelopes in the postbox. Useful to iterate through them
EnumerableSet.Bytes32Set postboxKeys;
/// @notice A mapping of block numbers to bottom-up cross-messages
// slither-disable-next-line uninitialized-state
mapping(uint256 => BottomUpMsgBatch) bottomUpMsgBatches;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ contract SubnetActorCheckpointingFacet is ISubnetActorCheckpointing, ReentrancyG
}

IGateway(LibSubnetActorStorage.appStorage().ipcGatewayAddr).execBottomUpMsgBatch(msgs);

// Propagate cross messages from checkpoint to other subnets
IGateway(LibSubnetActorStorage.appStorage().ipcGatewayAddr).propagateAll();
}

function ensureValidHeight(uint64 blockHeight, uint64 lastHeight) internal view {
Expand Down
4 changes: 2 additions & 2 deletions contracts/test/helpers/SelectorLibrary.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ library SelectorLibrary {
if (keccak256(abi.encodePacked(facetName)) == keccak256(abi.encodePacked("GatewayGetterFacet"))) {
return
abi.decode(
hex"000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000188789f83b0000000000000000000000000000000000000000000000000000000006c4685300000000000000000000000000000000000000000000000000000000dd81b5cf0000000000000000000000000000000000000000000000000000000041b6a2e80000000000000000000000000000000000000000000000000000000038d6693200000000000000000000000000000000000000000000000000000000444ead5100000000000000000000000000000000000000000000000000000000544dddff000000000000000000000000000000000000000000000000000000006ad21bb000000000000000000000000000000000000000000000000000000000b1ba49b000000000000000000000000000000000000000000000000000000000f3229131000000000000000000000000000000000000000000000000000000000338150f0000000000000000000000000000000000000000000000000000000094074b03000000000000000000000000000000000000000000000000000000007edeac9200000000000000000000000000000000000000000000000000000000c66c66a1000000000000000000000000000000000000000000000000000000003594c3c1000000000000000000000000000000000000000000000000000000009d3070b50000000000000000000000000000000000000000000000000000000042398a9a00000000000000000000000000000000000000000000000000000000fa34a400000000000000000000000000000000000000000000000000000000005d02968500000000000000000000000000000000000000000000000000000000599c7bd1000000000000000000000000000000000000000000000000000000008cfd78e7000000000000000000000000000000000000000000000000000000007474d79f0000000000000000000000000000000000000000000000000000000002e30f9a00000000000000000000000000000000000000000000000000000000a2b6715800000000000000000000000000000000000000000000000000000000",
hex"000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000168789f83b0000000000000000000000000000000000000000000000000000000006c4685300000000000000000000000000000000000000000000000000000000dd81b5cf0000000000000000000000000000000000000000000000000000000041b6a2e80000000000000000000000000000000000000000000000000000000038d6693200000000000000000000000000000000000000000000000000000000444ead5100000000000000000000000000000000000000000000000000000000544dddff000000000000000000000000000000000000000000000000000000006ad21bb000000000000000000000000000000000000000000000000000000000b1ba49b000000000000000000000000000000000000000000000000000000000f3229131000000000000000000000000000000000000000000000000000000000338150f0000000000000000000000000000000000000000000000000000000094074b03000000000000000000000000000000000000000000000000000000007edeac9200000000000000000000000000000000000000000000000000000000c66c66a1000000000000000000000000000000000000000000000000000000003594c3c1000000000000000000000000000000000000000000000000000000009d3070b50000000000000000000000000000000000000000000000000000000042398a9a00000000000000000000000000000000000000000000000000000000fa34a400000000000000000000000000000000000000000000000000000000005d02968500000000000000000000000000000000000000000000000000000000599c7bd10000000000000000000000000000000000000000000000000000000002e30f9a00000000000000000000000000000000000000000000000000000000a2b6715800000000000000000000000000000000000000000000000000000000",
(bytes4[])
);
}
Expand All @@ -41,7 +41,7 @@ library SelectorLibrary {
if (keccak256(abi.encodePacked(facetName)) == keccak256(abi.encodePacked("GatewayMessengerFacet"))) {
return
abi.decode(
hex"000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000027f7999f4000000000000000000000000000000000000000000000000000000002c85ec2c00000000000000000000000000000000000000000000000000000000",
hex"000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000012c85ec2c00000000000000000000000000000000000000000000000000000000",
(bytes4[])
);
}
Expand Down
6 changes: 1 addition & 5 deletions contracts/test/integration/GatewayDiamond.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,6 @@ contract GatewayActorDiamondTest is Test, IntegrationTestBase, SubnetWithNativeT
gatewayDiamond.getter().majorityPercentage() == DEFAULT_MAJORITY_PERCENTAGE,
"unexpected majorityPercentage"
);

IpcEnvelope memory storableMsg = gatewayDiamond.getter().postbox(0);
IpcEnvelope memory msg1;
require(msg1.toHash() == storableMsg.toHash(), "unexpected hash");
}

function testGatewayDiamond_NewGatewayWithDefaultParams() public view {
Expand Down Expand Up @@ -557,7 +553,7 @@ contract GatewayActorDiamondTest is Test, IntegrationTestBase, SubnetWithNativeT
}

function testGatewayDiamond_SendCrossMessage_Fails_Fuzz(uint256 fee) public {
vm.assume(fee < DEFAULT_CROSS_MSG_FEE);
vm.assume(fee > 0 && fee < DEFAULT_CROSS_MSG_FEE);

address caller = CHILD_NETWORK_ADDRESS;
vm.deal(caller, DEFAULT_COLLATERAL_AMOUNT + DEFAULT_CROSS_MSG_FEE + 2);
Expand Down
Loading
Loading