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

fix(world): check namespace exists before balance transfer [L-03] #2095

Merged
merged 12 commits into from
Jan 12, 2024
5 changes: 5 additions & 0 deletions .changeset/seven-carpets-develop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@latticexyz/world": patch
---

Namespace balances can no longer be transferred to non-existent namespaces.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity >=0.8.21;

import { ResourceId, ResourceIdInstance } from "@latticexyz/store/src/ResourceId.sol";
import { ResourceIds } from "@latticexyz/store/src/codegen/tables/ResourceIds.sol";

import { System } from "../../../System.sol";
import { revertWithBytes } from "../../../revertWithBytes.sol";
Expand Down Expand Up @@ -37,6 +38,9 @@ contract BalanceTransferSystem is System, IWorldErrors {
revert World_InvalidResourceType(RESOURCE_NAMESPACE, toNamespaceId, toNamespaceId.toString());
}

// Require the namespace to exist
AccessControl.requireExistence(toNamespaceId);

// Require caller to have access to the namespace
AccessControl.requireAccess(fromNamespaceId, _msgSender());
Copy link
Member

@holic holic Jan 10, 2024

Choose a reason for hiding this comment

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

I wonder how much gas we'd incur if we added this check in requireAccess instead?

edit: see #2105


Expand Down
26 changes: 26 additions & 0 deletions packages/world/test/WorldBalance.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,32 @@ contract WorldBalanceTest is Test, GasReporter {
assertEq(Balances.get(invalidNamespace), 0);
}

function testTransferBalanceToNamespaceRevertResourceNotFound() public {
bytes14 toNamespace = "not_registered";
ResourceId toNamespaceId = WorldResourceIdLib.encodeNamespace(toNamespace);

uint256 value = 1 ether;

// Expect the root namespace to have no balance
assertEq(Balances.get(ROOT_NAMESPACE_ID), 0);

// Send balance to root namespace
vm.deal(caller, value);
vm.prank(caller);
(bool success, bytes memory data) = address(world).call{ value: value }("");
assertTrue(success);
assertEq(data.length, 0);

// Expect the root namespace to have the value as balance
assertEq(Balances.get(ROOT_NAMESPACE_ID), value);

// Expect revert when attempting to transfer to a non-existent namespace
vm.expectRevert(
abi.encodeWithSelector(IWorldErrors.World_ResourceNotFound.selector, toNamespaceId, toNamespaceId.toString())
);
world.transferBalanceToNamespace(ROOT_NAMESPACE_ID, toNamespaceId, value);
}

function testTransferBalanceToAddress() public {
uint256 value = 1 ether;

Expand Down
Loading