Skip to content

Commit

Permalink
Merge pull request #176 from blackbeard002/del
Browse files Browse the repository at this point in the history 
refactor: lower gas cost of createSwap by delegating security checks to the implementer #172
  • Loading branch information
@0xneves
0xneves authored Jan 16, 2024
2 parents b7f6603 + aa61f3e commit 4f9458b
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 88 deletions.
9 changes: 1 addition & 8 deletions contracts/Swaplace.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@ contract Swaplace is SwapFactory, ISwaplace, IERC165 {
function createSwap(Swap calldata swap) public returns (uint256) {
if (swap.owner != msg.sender) revert InvalidAddress(msg.sender);

(address allowed, uint256 expiry) = parseData(swap.config);

if (expiry < block.timestamp) revert InvalidExpiry(expiry);

if (swap.biding.length == 0 || swap.asking.length == 0)
revert InvalidAssetsLength();

unchecked {
assembly {
sstore(_totalSwaps.slot, add(sload(_totalSwaps.slot), 1))
Expand All @@ -43,7 +36,7 @@ contract Swaplace is SwapFactory, ISwaplace, IERC165 {

_swaps[swapId] = swap;

emit SwapCreated(swapId, msg.sender, allowed, expiry);
emit SwapCreated(swapId);

return swapId;
}
Expand Down
5 changes: 1 addition & 4 deletions contracts/interfaces/ISwaplace.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,7 @@ interface ISwaplace {
* @dev Emitted when a new Swap is created.
*/
event SwapCreated(
uint256 indexed swapId,
address indexed owner,
address indexed allowed,
uint256 expiry
uint256 indexed swapId
);

/**
Expand Down
85 changes: 9 additions & 76 deletions test/TestSwaplace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
zeroAddress,
currentTimestamp,
);
.withArgs(await Swaplace.totalSwaps());
});

it("Should be able to create a 1-N swap with ERC20", async function () {
Expand Down Expand Up @@ -110,12 +105,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
zeroAddress,
currentTimestamp,
);
.withArgs(await Swaplace.totalSwaps());
});

it("Should be able to create a N-N swap with ERC20", async function () {
Expand Down Expand Up @@ -147,12 +137,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
zeroAddress,
currentTimestamp,
);
.withArgs(await Swaplace.totalSwaps());
});

it("Should be able to create a 1-1 swap with ERC721", async function () {
Expand All @@ -176,12 +161,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
zeroAddress,
currentTimestamp,
);
.withArgs(await Swaplace.totalSwaps());
});

it("Should be able to create a 1-N swap with ERC721", async function () {
Expand Down Expand Up @@ -209,12 +189,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
zeroAddress,
currentTimestamp,
);
.withArgs(await Swaplace.totalSwaps());
});

it("Should be able to create a N-N swap with ERC721", async function () {
Expand Down Expand Up @@ -246,12 +221,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
zeroAddress,
currentTimestamp,
);
.withArgs(await Swaplace.totalSwaps());
});
});

Expand All @@ -262,28 +232,6 @@ describe("Swaplace", async function () {
.to.be.revertedWithCustomError(Swaplace, `InvalidAddress`)
.withArgs(acceptee.address);
});

it("Should revert when {expiry} is smaller than {block.timestamp}", async function () {
const swap = await mockSwap();

let [allowed, expiry] = await Swaplace.parseData(swap.config);
expiry /= 2;
swap.config = await Swaplace.packData(allowed, expiry);

await expect(Swaplace.connect(owner).createSwap(swap))
.to.be.revertedWithCustomError(Swaplace, `InvalidExpiry`)
.withArgs(expiry);
});

it("Should revert when {biding} and {asking} lengths are equal 0", async function () {
const swap = await mockSwap();
swap.biding = [];
swap.asking = [];

await expect(
Swaplace.connect(owner).createSwap(swap),
).to.be.revertedWithCustomError(Swaplace, `InvalidAssetsLength`);
});
});
});

Expand Down Expand Up @@ -354,12 +302,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
allowed,
expiry,
);
.withArgs(await Swaplace.totalSwaps());

await expect(
await Swaplace.connect(acceptee).acceptSwap(
Expand All @@ -386,12 +329,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
allowed,
expiry,
);
.withArgs(await Swaplace.totalSwaps());

await expect(
await Swaplace.connect(acceptee).acceptSwap(
Expand Down Expand Up @@ -472,12 +410,7 @@ describe("Swaplace", async function () {

await expect(await Swaplace.connect(owner).createSwap(swap))
.to.emit(Swaplace, "SwapCreated")
.withArgs(
await Swaplace.totalSwaps(),
owner.address,
allowed,
expiry,
);
.withArgs(await Swaplace.totalSwaps());

await expect(
Swaplace.connect(acceptee).acceptSwap(
Expand Down

0 comments on commit 4f9458b

Please sign in to comment.