Skip to content

Commit

Permalink
test: create test suite for core functionality of LSP17Extendable
Browse files Browse the repository at this point in the history
  • Loading branch information
CJ42 committed Aug 11, 2023
1 parent ada5adf commit 5cd442f
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 3 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build-lint-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ jobs:
"lsp9init",
"lsp11",
"lsp11init",
"lsp17",
"lsp20",
"lsp20init",
"lsp23",
Expand Down
1 change: 1 addition & 0 deletions contracts/LSP17ContractExtension/LSP17Extendable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ abstract contract LSP17Extendable is ERC165 {
} else {
// `result` -> first word in memory where the length of `result` is stored
// `add(result, 32)` -> next word in memory is where the `result` data starts
// solhint-disable no-inline-assembly
assembly {
revert(add(result, 32), mload(result))
}
Expand Down
1 change: 1 addition & 0 deletions contracts/LSP9Vault/LSP9VaultCore.sol
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ contract LSP9VaultCore is
} else {
// `result` -> first word in memory where the length of `result` is stored
// `add(result, 32)` -> next word in memory is where the `result` data starts
// solhint-disable no-inline-assembly
assembly {
revert(add(result, 32), mload(result))
}
Expand Down
31 changes: 31 additions & 0 deletions contracts/Mocks/FallbackExtensions/RevertErrorsTestExtension.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.0;

/**
* @dev This contract is used only for testing purposes
*/
contract RevertErrorsTestExtension {
error SomeCustomError(address someAddress);

function revertWithCustomError() public view {
revert SomeCustomError(msg.sender);
}

function revertWithErrorString() public pure {
revert("some error message");
}

function revertWithPanicError() public pure {
uint256 number = 2;

// trigger an arithmetic underflow.
// this should trigger a error of type `Panic(uint256)`
// with error code 17 (0x11) --> Panic(0x11)
number -= 10;
}

function revertWithNoErrorData() public pure {
// solhint-disable reason-string
revert();
}
}
68 changes: 68 additions & 0 deletions contracts/Mocks/LSP17ExtendableTester.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.4;

import {LSP17Extendable} from "../LSP17ContractExtension/LSP17Extendable.sol";

/**
* @dev This contract is used only for testing purposes
*/
contract LSP17ExtendableTester is LSP17Extendable {
mapping(bytes4 => address) internal _extensions;

string internal _someStorageData;
string internal _anotherStorageData;

// This `receive()` function is just put there to disable the following solc compiler warning:
//
// "This contract has a payable fallback function, but no receive ether function.
// Consider adding a receive ether function."
receive() external payable {}

// solhint-disable no-complex-fallback
fallback() external payable {
// CHECK we can update the contract's storage BEFORE calling an extension
setStorageData("updated BEFORE calling `_fallbackLSP17Extendable`");

_fallbackLSP17Extendable(msg.data);

// CHECK we can update the contract's storage AFTER calling an extension
setAnotherStorageData(
"updated AFTER calling `_fallbackLSP17Extendable`"
);
}

function getExtension(
bytes4 functionSelector
) public view returns (address) {
return _getExtension(functionSelector);
}

function setExtension(
bytes4 functionSelector,
address extensionContract
) public {
_extensions[functionSelector] = extensionContract;
}

function getStorageData() public view returns (string memory) {
return _someStorageData;
}

function setStorageData(string memory newData) public {
_someStorageData = newData;
}

function getAnotherStorageData() public view returns (string memory) {
return _anotherStorageData;
}

function setAnotherStorageData(string memory newData) public {
_anotherStorageData = newData;
}

function _getExtension(
bytes4 functionSelector
) internal view override returns (address) {
return _extensions[functionSelector];
}
}
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"test:lsp9init": "hardhat test --no-compile tests/LSP9Vault/LSP9VaultInit.test.ts",
"test:lsp11": "hardhat test --no-compile tests/LSP11BasicSocialRecovery/LSP11BasicSocialRecovery.test.ts",
"test:lsp11init": "hardhat test --no-compile tests/LSP11BasicSocialRecovery/LSP11BasicSocialRecoveryInit.test.ts",
"test:lsp17": "hardhat test --no-compile tests/LSP17ContractExtension/LSP17Extendable.test.ts",
"test:lsp20": "hardhat test --no-compile tests/LSP20CallVerification/LSP6/LSP20WithLSP6.test.ts",
"test:lsp20init": "hardhat test --no-compile tests/LSP20CallVerification/LSP6/LSP20WithLSP6Init.test.ts",
"test:lsp23": "hardhat test --no-compile tests/LSP23LinkedContractsDeployment/LSP23LinkedContractsDeployment.test.ts",
Expand Down
13 changes: 10 additions & 3 deletions tests/LSP17ContractExtension/LSP17Extendable.behaviour.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
.withArgs(checkMsgVariableFunctionSelector);
});

it('should pass even if passed a different value from the msg.value', async () => {
it('should revert with NoExtensionFoundForFunctionSelector, even if passed a different value from the msg.value', async () => {
const sender = context.accounts[0];
const supposedValue = 200;
const checkMsgVariableFunctionSignature =
Expand Down Expand Up @@ -667,6 +667,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
.connect(context.deployParams.owner)
.setData(bytes1ZeroPaddedExtensionHandlerKey, revertFallbackExtension.address);
});

it('should pass even if there is an extension for it that reverts', async () => {
await expect(
context.accounts[0].sendTransaction({
Expand All @@ -677,7 +678,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
});
});

describe('when calling with a payload preprended with 4 bytes of 0', () => {
describe('when calling with a payload prepended with 4 bytes of 0', () => {
describe('when no extension is set for bytes4(0)', () => {
describe('when the payload is `0x00000000`', () => {
describe('with sending value', () => {
Expand All @@ -694,6 +695,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
.withArgs(context.accounts[0].address, amountSent);
});
});

describe('without sending value', () => {
it('should pass', async () => {
await expect(
Expand All @@ -705,6 +707,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
});
});
});

describe("when the payload is `0x00000000` + some random data ('graffiti')", () => {
describe('with sending value', () => {
it('should pass and emit ValueReceived value', async () => {
Expand All @@ -726,6 +729,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
.withArgs(context.accounts[0].address, amountSent);
});
});

describe('without sending value', () => {
it('should pass', async () => {
const graffiti =
Expand All @@ -744,6 +748,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
});
});
});

describe('when there is an extension set for bytes4(0)', () => {
describe('when setting an extension that reverts', () => {
let revertFallbackExtension: RevertFallbackExtension;
Expand All @@ -762,6 +767,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
.connect(context.deployParams.owner)
.setData(bytes1ZeroPaddedExtensionHandlerKey, revertFallbackExtension.address);
});

describe('when the payload is `0x00000000`', () => {
it('should revert', async () => {
await expect(
Expand All @@ -772,6 +778,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
).to.be.reverted;
});
});

describe("when the payload is `0x00000000` + some random data ('graffiti')", () => {
it('should revert', async () => {
const graffiti =
Expand Down Expand Up @@ -801,7 +808,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise<LSP17TestConte
token = await new RequireCallbackToken__factory(context.accounts[0]).deploy();
});

describe('when minitng to the account', () => {
describe('when minting to the account', () => {
describe('before setting the onERC721ReceivedExtension', () => {
it('should fail since onERC721Received is not implemented', async () => {
await expect(token.mint(context.contract.address)).to.be.reverted;
Expand Down
144 changes: 144 additions & 0 deletions tests/LSP17ContractExtension/LSP17Extendable.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { expect } from 'chai';
import { ethers } from 'hardhat';

import {
LSP17ExtendableTester,
LSP17ExtendableTester__factory,
EmitEventExtension,
EmitEventExtension__factory,
RevertErrorsTestExtension,
RevertErrorsTestExtension__factory,
} from '../../types';

describe('LSP17Extendable - Basic Implementation', () => {
let accounts;

let lsp17Implementation: LSP17ExtendableTester;
let exampleExtension: EmitEventExtension;
let errorsExtension: RevertErrorsTestExtension;

const selectorWithExtension =
EmitEventExtension__factory.createInterface().getSighash('emitEvent');
const selectorWithNoExtension = '0xdeadbeef';

// selectors to test that errors are bubbled up to the contract
const revertErrorsExtensionInterface = RevertErrorsTestExtension__factory.createInterface();

const selectorRevertCustomError =
revertErrorsExtensionInterface.getSighash('revertWithCustomError');

const selectorRevertErrorString =
revertErrorsExtensionInterface.getSighash('revertWithErrorString');

const selectorRevertPanicError =
revertErrorsExtensionInterface.getSighash('revertWithPanicError');

const selectorRevertNoErrorData =
revertErrorsExtensionInterface.getSighash('revertWithNoErrorData');

before('setup', async () => {
accounts = await ethers.getSigners();

lsp17Implementation = await new LSP17ExtendableTester__factory(accounts[0]).deploy();
exampleExtension = await new EmitEventExtension__factory(accounts[0]).deploy();
errorsExtension = await new RevertErrorsTestExtension__factory(accounts[0]).deploy();

await lsp17Implementation.setExtension(selectorWithExtension, exampleExtension.address);

await lsp17Implementation.setExtension(selectorRevertCustomError, errorsExtension.address);
await lsp17Implementation.setExtension(selectorRevertErrorString, errorsExtension.address);
await lsp17Implementation.setExtension(selectorRevertPanicError, errorsExtension.address);
await lsp17Implementation.setExtension(selectorRevertNoErrorData, errorsExtension.address);
});

// make sure storage is cleared before running each test
afterEach(async () => {
await lsp17Implementation.setStorageData('0x');
await lsp17Implementation.setAnotherStorageData('0x');
});

describe('when there is no extension set', () => {
it('should revert with error `NoExtensionFoundForFunctionSelector', async () => {
await expect(
accounts[0].sendTransaction({
to: lsp17Implementation.address,
data: selectorWithNoExtension,
}),
).to.be.revertedWithCustomError(lsp17Implementation, 'NoExtensionFoundForFunctionSelector');
});
});

describe('when there is an extension set', () => {
describe('if the extension does not revert', () => {
it('should pass and not revert', async () => {
await expect(
accounts[0].sendTransaction({
to: lsp17Implementation.address,
data: selectorWithExtension,
}),
).to.emit(exampleExtension, 'EventEmittedInExtension');
});

describe('if there is any code logic that run after extension was called', () => {
it("should have updated the contract's storage after the fallback LSP17 function ran", async () => {
const storageBefore = await lsp17Implementation.getStorageData();
expect(storageBefore).to.equal('0x');

const anotherStorageBefore = await lsp17Implementation.getAnotherStorageData();
expect(anotherStorageBefore).to.equal('0x');

await accounts[0].sendTransaction({
to: lsp17Implementation.address,
data: selectorWithExtension,
});

const storageAfter = await lsp17Implementation.getStorageData();
expect(storageAfter).to.equal('updated BEFORE calling `_fallbackLSP17Extendable`');

const anotherStorageAfter = await lsp17Implementation.getAnotherStorageData();
expect(anotherStorageAfter).to.equal('updated AFTER calling `_fallbackLSP17Extendable`');
});
});
});

describe('if the extension revert', () => {
it('should bubble up custom errors', async () => {
await expect(
accounts[0].sendTransaction({
to: lsp17Implementation.address,
data: selectorRevertCustomError,
}),
)
.to.be.revertedWithCustomError(errorsExtension, 'SomeCustomError')
.withArgs(lsp17Implementation.address);
});

it('should bubble up revert errors string', async () => {
await expect(
accounts[0].sendTransaction({
to: lsp17Implementation.address,
data: selectorRevertErrorString,
}),
).to.be.revertedWith('some error message');
});

it('should bubble up Panic type errors with their code', async () => {
await expect(
accounts[0].sendTransaction({
to: lsp17Implementation.address,
data: selectorRevertPanicError,
}),
).to.be.revertedWithPanic('0x11' || 17);
});

it('should not bubble up anything with empty error data (`revert()`)', async () => {
await expect(
accounts[0].sendTransaction({
to: lsp17Implementation.address,
data: selectorRevertNoErrorData,
}),
).to.be.reverted;
});
});
});
});

0 comments on commit 5cd442f

Please sign in to comment.