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.

Sign up for GitHub

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

Add custom errors and address masking. Fix update data logic #3

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add custom errors and address masking. Fix update data logic #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 40 additions & 30 deletions src/Repository.huff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@
STORE_ELEMENT_FROM_KEYS(0x00) // [dpd_id]

// Store the address of the owner.
0x24 calldataload // [owner, dpd_id, length*32, length]
0x24 calldataload MASK_ADDRESS() // [owner, dpd_id, length*32, length]
dup2 // [dpd_id, owner, dpd_io, length*32, length]
[DPD_OWNER_MAP_SLOT] // [salt, dpd_id, owner, dpd_io, length*32, length]
STORE_ELEMENT_FROM_KEYS(0x00) // [dpd_id, length*32, length]

// Store the address of the updater.
0x44 calldataload // [updater, dpd_id, length*32, length]
0x44 calldataload MASK_ADDRESS() // [updater, dpd_id, length*32, length]
dup2 // [dpd_id, updater, dpd_id, length*32, length]
[DPD_UPDATER_MAP_SLOT] // [salt, dpd_id, updater, dpd_id, length*32, length]
STORE_ELEMENT_FROM_KEYS(0x00) // [dpd_id, length*32, length]
Expand All @@ -107,7 +107,29 @@

// Revert if the checks are wrong.
error:
0x00 0x00 revert
0x23369fa6 0x00 mstore
0x04 0x1C revert // Revert with custom error AlreadyExists()
}

#define macro IS_OWNER(error) = takes(0) returns (0) {
// Ensure that the sender is the DPD owner.
[DPD_OWNER_MAP_SLOT] // [salt, dpd_id, dpd_id, updater]
LOAD_ELEMENT_FROM_KEYS(0x00) // [owner, dpd_id, updater]
caller // [msg.sender, owner, dpd_id, updater]
eq iszero // [msg.sender != owner, dpd_id, updater]
}

#define macro IS_UPDATER(error) = takes(0) returns (0) {
// Ensure that the sender is the DPD owner.
[DPD_UPDATER_MAP_SLOT] // [salt, dpd_id, dpd_id, updater]
LOAD_ELEMENT_FROM_KEYS(0x00) // [owner, dpd_id, updater]
caller // [msg.sender, owner, dpd_id, updater]
eq iszero // [msg.sender != owner, dpd_id, updater]
}

#define macro ONLY_OWNER(error) = takes(0) returns (0) {
IS_OWNER(error)
<error> jumpi // [dpd_id, updater]
}

// Ensure that a DPD ID has not been taken.
Expand Down Expand Up @@ -152,17 +174,11 @@
log2 // [dpd_id, data]

// Ensure that the sender is the DPD updater or owner.
dup1 // [dpd_id, dpd_id, data]
[DPD_UPDATER_MAP_SLOT] // [salt, dpd_id, dpd_id, data]
LOAD_ELEMENT_FROM_KEYS(0x00) // [updater, dpd_id, data]
caller // [msg.sender, updater, dpd_id, data]
eq iszero // [msg.sender!=updater, dpd_id, data]
dup1
IS_UPDATER() // [msg.sender!=updater, dpd_id, data]
dup2 // [dpd_id, msg.sender!=updater, dpd_id, data]
[DPD_OWNER_MAP_SLOT] // [salt, dpd_id, msg.sender!=updater, dpd_id, data]
LOAD_ELEMENT_FROM_KEYS(0x00) // [owner, msg.sender!=updater, dpd_id, data]
caller // [msg.sender, owner, msg.sender!=updater, dpd_id, data]
eq iszero // [msg.sender!=owner, msg.sender!=updater, dpd_id, data]
or // [msg.sender!=owner || msg.sender!=updater, dpd_id, data]
IS_OWNER() // [msg.sender!=owner, msg.sender!=updater, dpd_id, data]
and // [msg.sender!=owner && msg.sender!=updater, dpd_id, data]
error jumpi // [dpd_id, data]

// Store new data.
Expand All @@ -174,9 +190,11 @@

// Error label.
error:
0x00 0x00 revert
0x9bb09934 0x00 mstore
0x04 0x1C revert // Revert with error OnlyOwnerOrUpdater()
}


#define macro UPDATE_DPD_UPDATER() = takes(0) returns (0) {
// Retrieve the DPD ID and data.
0x24 calldataload // [updater]
Expand All @@ -190,15 +208,8 @@
0x00 mstore // [sig, dpd_id, dpd_id, updater]
0x20 0x00 // [offset, size, sig, dpd_id, dpd_id, updater]
log2 // [dpd_id, updater]


// Ensure that the sender is the DPD owner.
dup1 // [dpd_id, dpd_id, updater]
[DPD_OWNER_MAP_SLOT] // [salt, dpd_id, dpd_id, updater]
LOAD_ELEMENT_FROM_KEYS(0x00) // [owner, dpd_id, updater]
caller // [msg.sender, owner, dpd_id, updater]
eq iszero // [msg.sender != owner, dpd_id, updater]
error jumpi // [dpd_id, updater]
ONLY_OWNER(error) // [dpd_id, updater]

// Store new data.
[DPD_UPDATER_MAP_SLOT] // [salt, dpd_id, updater]
Expand All @@ -209,12 +220,14 @@

// Error label.
error:
0x00 0x00 revert
0x5fc483c5 0x00 mstore
0x04 0x1C revert // Revert with custom error OnlyOwner()

}

#define macro UPDATE_DPD_OWNER() = takes(0) returns (0) {
// Retrieve the DPD ID and data.
0x24 calldataload // [owner]
0x24 calldataload MASK_ADDRESS() // [owner]
0x04 calldataload // [dpd_id, owner]

// Emit the event.
Expand All @@ -228,11 +241,7 @@

// Ensure that the sender is the DPD owner.
dup1 // [dpd_id, dpd_id, owner]
[DPD_OWNER_MAP_SLOT] // [salt, dpd_id, dpd_id, owner]
LOAD_ELEMENT_FROM_KEYS(0x00) // [owner, dpd_id, owner]
caller // [msg.sender, owner, dpd_id, owner]
eq iszero // [msg.sender != owner, dpd_id, owner]
error jumpi // [dpd_id, owner]
ONLY_OWNER(error) // [dpd_id, owner]

// Store new data.
[DPD_OWNER_MAP_SLOT] // [salt, dpd_id, owner]
Expand All @@ -243,7 +252,8 @@

// Error label.
error:
0x00 0x00 revert
0x5fc483c5 0x00 mstore
0x04 0x1C revert // Revert with custom error OnlyOwner()
}

/* Main Macro */
Expand Down
70 changes: 66 additions & 4 deletions test/Repository.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,16 +85,25 @@ contract RepositoryTest is Test {
}

/// @dev Ensure that you cannot create two DPDs with the same ID.
function testFailNumberMustBeDifferent() public {
function testNumberMustBeDifferent() public {
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
vm.expectRevert(Repository.AlreadyExists.selector);
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
}

/// @dev Ensure that non owner cannot update data
function testUpdateDataNoAuth() public {
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
vm.prank(address(20));
vm.expectRevert(Repository.OnlyOwnerOrUpdater.selector);
repository.updateDpdData(0, bytes32(uint256(1000)));
}

/// @dev Ensure that you can update DPD data.
function testUpdateData() public {
function testUpdateDataAsOwner() public {
vm.expectEmit(true, true, true, true);
emit DPDAdded(0, address(this), address(this), bytes32(uint256(69)));
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
emit DPDAdded(0, address(this), address(20), bytes32(uint256(69)));
repository.addDpd(0, address(this), address(20), bytes32(uint256(69)));

vm.expectEmit(true, true, true, true);
emit DPDUpdated(0, bytes32(uint256(1000)));
Expand All @@ -103,6 +112,27 @@ contract RepositoryTest is Test {
assertEq(uint256(repository.dpds(0)), 1000);
}

/// @dev Ensure that you can update DPD data.
function testUpdateDataAsUpdater() public {
vm.expectEmit(true, true, true, true);
emit DPDAdded(0, address(20), address(this), bytes32(uint256(69)));
repository.addDpd(0, address(20), address(this), bytes32(uint256(69)));

vm.expectEmit(true, true, true, true);
emit DPDUpdated(0, bytes32(uint256(1000)));
repository.updateDpdData(0, bytes32(uint256(1000)));

assertEq(uint256(repository.dpds(0)), 1000);
}

/// @dev Ensure that non owner account cant update the updater address of a DPD.
function testUpdateUpdaterNoAuth() public {
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
vm.prank(address(20));
vm.expectRevert(Repository.OnlyOwner.selector);
repository.updateDpdUpdater(0, address(20));
}

/// @dev Ensure that you can update the updater address of a DPD.
function testUpdateUpdater() public {
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
Expand All @@ -114,6 +144,14 @@ contract RepositoryTest is Test {
assertEq(repository.updater(0), address(20));
}

/// @dev Ensure that a non owner account cannot update the owner address of a DPD
function testUpdateOwnerNoAuth() public {
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
vm.prank(address(20));
vm.expectRevert(Repository.OnlyOwner.selector);
repository.updateDpdOwner(0, address(20));
}

/// @dev Ensure that you can update the owner address of a DPD.
function testUpdateOwner() public {
repository.addDpd(0, address(this), address(this), bytes32(uint256(69)));
Expand All @@ -124,6 +162,24 @@ contract RepositoryTest is Test {

assertEq(repository.owner(0), address(20));
}

function testFuzzAddDpd(uint id, address owner, address owner2, address updater, address updater2, bytes32 cid, bytes32 cid2) public {
repository.addDpd(id, owner, updater, cid);
assertEq(repository.dpds(id), cid);

vm.prank(updater);
repository.updateDpdData(id, cid2);

vm.prank(owner);
repository.updateDpdOwner(id, owner2);

vm.prank(owner2);
repository.updateDpdUpdater(id, updater2);

assertEq(repository.dpds(id), cid2);
assertEq(repository.updater(id), updater2);
assertEq(repository.owner(id), owner2);
}
}

interface Repository {
Expand Down Expand Up @@ -160,4 +216,10 @@ interface Repository {

/// @notice Event emitted when a DPD's upgrader is changed.
event DPDUpdaterUpdated(uint256 indexed id, address newUpdater);

error AlreadyExists();

error OnlyOwner();

error OnlyOwnerOrUpdater();
}