From b7a0bad40b1c5f774169e6917daaa4df2606f2b4 Mon Sep 17 00:00:00 2001 From: CJ42 Date: Thu, 10 Aug 2023 16:26:41 +0100 Subject: [PATCH 1/2] fix: allow running code after `_fallbackLSP17Extendable` function. --- .../LSP0ERC725AccountCore.sol | 70 ++++++-------- .../LSP17Extendable.sol | 51 ++++------ contracts/LSP9Vault/LSP9VaultCore.sol | 92 +++++++++---------- 3 files changed, 85 insertions(+), 128 deletions(-) diff --git a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol index ecf7a4473..61b8206ea 100644 --- a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol +++ b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol @@ -152,16 +152,18 @@ abstract contract LSP0ERC725AccountCore is * * @custom:events {ValueReceived} event when receiving native tokens. */ - fallback() external payable virtual { + fallback( + bytes calldata callData + ) external payable virtual returns (bytes memory) { if (msg.value != 0) { emit ValueReceived(msg.sender, msg.value); } if (msg.data.length < 4) { - return; + return ""; } - _fallbackLSP17Extendable(); + return _fallbackLSP17Extendable(callData); } /** @@ -789,57 +791,37 @@ abstract contract LSP0ERC725AccountCore is * If there is an extension for the function selector being called, it calls the extension with the * CALL opcode, passing the `msg.data` appended with the 20 bytes of the `msg.sender` and * 32 bytes of the `msg.value` - * - * Because the function uses assembly `return()`/`revert()` to terminate the call, it cannot be - * called before other codes in {fallback()}. - * - * Otherwise, the codes after {_fallbackLSP17Extendable()} may never be reached. */ - function _fallbackLSP17Extendable() internal virtual override { + function _fallbackLSP17Extendable( + bytes calldata callData + ) internal virtual override returns (bytes memory) { // If there is a function selector address extension = _getExtension(msg.sig); // if no extension was found for bytes4(0) return don't revert - if (msg.sig == bytes4(0) && extension == address(0)) return; + if (msg.sig == bytes4(0) && extension == address(0)) return ""; // if no extension was found for other function selectors, revert if (extension == address(0)) revert NoExtensionFoundForFunctionSelector(msg.sig); - // solhint-disable no-inline-assembly - // if the extension was found, call the extension with the msg.data - // appended with bytes20(address) and bytes32(msg.value) - assembly { - calldatacopy(0, 0, calldatasize()) - - // The msg.sender address is shifted to the left by 12 bytes to remove the padding - // Then the address without padding is stored right after the calldata - mstore(calldatasize(), shl(96, caller())) - - // The msg.value is stored right after the calldata + msg.sender - mstore(add(calldatasize(), 20), callvalue()) - - // Add 52 bytes for the msg.sender and msg.value appended at the end of the calldata - let success := call( - gas(), - extension, - 0, - 0, - add(calldatasize(), 52), - 0, - 0 - ) - - // Copy the returned data - returndatacopy(0, 0, returndatasize()) - - switch success - // call returns 0 on failed calls - case 0 { - revert(0, returndatasize()) - } - default { - return(0, returndatasize()) + bytes memory calldataWithCallerInfos = abi.encodePacked( + callData, + msg.sender, + msg.value + ); + + (bool success, bytes memory result) = extension.call( + calldataWithCallerInfos + ); + + if (success) { + return result; + } 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 + assembly { + revert(add(result, 32), mload(result)) } } } diff --git a/contracts/LSP17ContractExtension/LSP17Extendable.sol b/contracts/LSP17ContractExtension/LSP17Extendable.sol index 66a5505cf..c5461988b 100644 --- a/contracts/LSP17ContractExtension/LSP17Extendable.sol +++ b/contracts/LSP17ContractExtension/LSP17Extendable.sol @@ -83,7 +83,9 @@ abstract contract LSP17Extendable is ERC165 { * * Otherwise, the codes after _fallbackLSP17Extendable() may never be reached. */ - function _fallbackLSP17Extendable() internal virtual { + function _fallbackLSP17Extendable( + bytes calldata callData + ) internal virtual returns (bytes memory) { // If there is a function selector address extension = _getExtension(msg.sig); @@ -91,40 +93,23 @@ abstract contract LSP17Extendable is ERC165 { if (extension == address(0)) revert NoExtensionFoundForFunctionSelector(msg.sig); - // solhint-disable no-inline-assembly - // if the extension was found, call the extension with the msg.data - // appended with bytes20(address) and bytes32(msg.value) - assembly { - calldatacopy(0, 0, calldatasize()) - - // The msg.sender address is shifted to the left by 12 bytes to remove the padding - // Then the address without padding is stored right after the calldata - mstore(calldatasize(), shl(96, caller())) - - // The msg.value is stored right after the calldata + msg.sender - mstore(add(calldatasize(), 20), callvalue()) - - // Add 52 bytes for the msg.sender and msg.value appended at the end of the calldata - let success := call( - gas(), - extension, - 0, - 0, - add(calldatasize(), 52), - 0, - 0 - ) + bytes memory calldataWithCallerInfos = abi.encodePacked( + callData, + msg.sender, + msg.value + ); - // Copy the returned data - returndatacopy(0, 0, returndatasize()) + (bool success, bytes memory result) = extension.call( + calldataWithCallerInfos + ); - switch success - // call returns 0 on failed calls - case 0 { - revert(0, returndatasize()) - } - default { - return(0, returndatasize()) + if (success) { + return result; + } 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 + assembly { + revert(add(result, 32), mload(result)) } } } diff --git a/contracts/LSP9Vault/LSP9VaultCore.sol b/contracts/LSP9Vault/LSP9VaultCore.sol index ee5fcc9ab..0a41223e4 100644 --- a/contracts/LSP9Vault/LSP9VaultCore.sol +++ b/contracts/LSP9Vault/LSP9VaultCore.sol @@ -116,73 +116,63 @@ contract LSP9VaultCore is * - the first 4 bytes of the calldata do not match any publicly callable functions from the contract ABI. * - receiving native tokens with some calldata. */ - fallback() external payable virtual { - if (msg.value != 0) emit ValueReceived(msg.sender, msg.value); - if (msg.data.length < 4) return; + fallback( + bytes calldata callData + ) external payable virtual returns (bytes memory) { + if (msg.value != 0) { + emit ValueReceived(msg.sender, msg.value); + } + + if (msg.data.length < 4) { + return ""; + } - _fallbackLSP17Extendable(); + return _fallbackLSP17Extendable(callData); } /** - * @dev Forwards the call to an extension mapped to a function selector. If no extension address - * is mapped to the function selector (address(0)), then revert. + * @dev Forwards the call to an extension mapped to a function selector. * - * The bytes4(0) msg.sig is an exception, the function won't revert if there is no extension found - * mapped to bytes4(0), but will execute the call to the extension in case it existed. - * - * The call to the extension is appended with bytes20 (msg.sender) and bytes32 (msg.value). - * Returns the return value on success and revert in case of failure. + * Calls {_getExtension} to get the address of the extension mapped to the function selector being + * called on the account. If there is no extension, the `address(0)` will be returned. * - * Because the function uses assembly {return()/revert()} to terminate the call, it cannot be - * called before other codes in fallback(). + * Reverts if there is no extension for the function being called, except for the bytes4(0) function + * selector, which passes even if there is no extension for it. * - * Otherwise, the codes after _fallbackLSP17Extendable() may never be reached. + * If there is an extension for the function selector being called, it calls the extension with the + * CALL opcode, passing the `msg.data` appended with the 20 bytes of the `msg.sender` and + * 32 bytes of the `msg.value` */ - function _fallbackLSP17Extendable() internal virtual override { + function _fallbackLSP17Extendable( + bytes calldata callData + ) internal virtual override returns (bytes memory) { // If there is a function selector address extension = _getExtension(msg.sig); // if no extension was found for bytes4(0) return don't revert - if (msg.sig == bytes4(0) && extension == address(0)) return; + if (msg.sig == bytes4(0) && extension == address(0)) return ""; - // if no extension was found, revert + // if no extension was found for other function selectors, revert if (extension == address(0)) revert NoExtensionFoundForFunctionSelector(msg.sig); - // solhint-disable no-inline-assembly - // if the extension was found, call the extension with the msg.data - // appended with bytes20(address) and bytes32(msg.value) - assembly { - calldatacopy(0, 0, calldatasize()) - - // The msg.sender address is shifted to the left by 12 bytes to remove the padding - // Then the address without padding is stored right after the calldata - mstore(calldatasize(), shl(96, caller())) - - // The msg.value is stored right after the calldata + msg.sender - mstore(add(calldatasize(), 20), callvalue()) - - // Add 52 bytes for the msg.sender and msg.value appended at the end of the calldata - let success := call( - gas(), - extension, - 0, - 0, - add(calldatasize(), 52), - 0, - 0 - ) - - // Copy the returned data - returndatacopy(0, 0, returndatasize()) - - switch success - // call returns 0 on failed calls - case 0 { - revert(0, returndatasize()) - } - default { - return(0, returndatasize()) + bytes memory calldataWithCallerInfos = abi.encodePacked( + callData, + msg.sender, + msg.value + ); + + (bool success, bytes memory result) = extension.call( + calldataWithCallerInfos + ); + + if (success) { + return result; + } 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 + assembly { + revert(add(result, 32), mload(result)) } } } From bf16c54e99f8fc5abf9607c5d8279c82330368f5 Mon Sep 17 00:00:00 2001 From: CJ42 Date: Fri, 11 Aug 2023 13:08:43 +0100 Subject: [PATCH 2/2] test: create test suite for core functionality of `LSP17Extendable` --- .github/workflows/build-lint-test.yml | 1 + .../LSP0ERC725AccountCore.sol | 15 +- .../LSP17Extendable.sol | 17 +-- contracts/LSP9Vault/LSP9VaultCore.sol | 17 +-- .../RevertErrorsTestExtension.sol | 31 ++++ contracts/Mocks/LSP17ExtendableTester.sol | 68 +++++++++ package.json | 1 + .../LSP17Extendable.behaviour.ts | 13 +- .../LSP17Extendable.test.ts | 144 ++++++++++++++++++ 9 files changed, 274 insertions(+), 33 deletions(-) create mode 100644 contracts/Mocks/FallbackExtensions/RevertErrorsTestExtension.sol create mode 100644 contracts/Mocks/LSP17ExtendableTester.sol create mode 100644 tests/LSP17ContractExtension/LSP17Extendable.test.ts diff --git a/.github/workflows/build-lint-test.yml b/.github/workflows/build-lint-test.yml index 435d99d21..223d60b02 100644 --- a/.github/workflows/build-lint-test.yml +++ b/.github/workflows/build-lint-test.yml @@ -64,6 +64,7 @@ jobs: "lsp9init", "lsp11", "lsp11init", + "lsp17", "lsp20", "lsp20init", "lsp23", diff --git a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol index 61b8206ea..754e6a43d 100644 --- a/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol +++ b/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol @@ -805,23 +805,18 @@ abstract contract LSP0ERC725AccountCore is if (extension == address(0)) revert NoExtensionFoundForFunctionSelector(msg.sig); - bytes memory calldataWithCallerInfos = abi.encodePacked( - callData, - msg.sender, - msg.value - ); - (bool success, bytes memory result) = extension.call( - calldataWithCallerInfos + abi.encodePacked(callData, msg.sender, msg.value) ); if (success) { return result; } 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 + // `mload(result)` -> offset in memory where `result.length` is located + // `add(result, 32)` -> offset in memory where `result` data starts assembly { - revert(add(result, 32), mload(result)) + let resultdata_size := mload(result) + revert(add(result, 32), resultdata_size) } } } diff --git a/contracts/LSP17ContractExtension/LSP17Extendable.sol b/contracts/LSP17ContractExtension/LSP17Extendable.sol index c5461988b..075f53711 100644 --- a/contracts/LSP17ContractExtension/LSP17Extendable.sol +++ b/contracts/LSP17ContractExtension/LSP17Extendable.sol @@ -93,23 +93,20 @@ abstract contract LSP17Extendable is ERC165 { if (extension == address(0)) revert NoExtensionFoundForFunctionSelector(msg.sig); - bytes memory calldataWithCallerInfos = abi.encodePacked( - callData, - msg.sender, - msg.value - ); - (bool success, bytes memory result) = extension.call( - calldataWithCallerInfos + abi.encodePacked(callData, msg.sender, msg.value) ); if (success) { return result; } 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 + // `mload(result)` -> offset in memory where `result.length` is located + // `add(result, 32)` -> offset in memory where `result` data starts + // solhint-disable no-inline-assembly + /// @solidity memory-safe-assembly assembly { - revert(add(result, 32), mload(result)) + let resultdata_size := mload(result) + revert(add(result, 32), resultdata_size) } } } diff --git a/contracts/LSP9Vault/LSP9VaultCore.sol b/contracts/LSP9Vault/LSP9VaultCore.sol index 0a41223e4..3e6a3fdba 100644 --- a/contracts/LSP9Vault/LSP9VaultCore.sol +++ b/contracts/LSP9Vault/LSP9VaultCore.sol @@ -156,23 +156,20 @@ contract LSP9VaultCore is if (extension == address(0)) revert NoExtensionFoundForFunctionSelector(msg.sig); - bytes memory calldataWithCallerInfos = abi.encodePacked( - callData, - msg.sender, - msg.value - ); - (bool success, bytes memory result) = extension.call( - calldataWithCallerInfos + abi.encodePacked(callData, msg.sender, msg.value) ); if (success) { return result; } 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 + // `mload(result)` -> offset in memory where `result.length` is located + // `add(result, 32)` -> offset in memory where `result` data starts + // solhint-disable no-inline-assembly + /// @solidity memory-safe-assembly assembly { - revert(add(result, 32), mload(result)) + let resultdata_size := mload(result) + revert(add(result, 32), resultdata_size) } } } diff --git a/contracts/Mocks/FallbackExtensions/RevertErrorsTestExtension.sol b/contracts/Mocks/FallbackExtensions/RevertErrorsTestExtension.sol new file mode 100644 index 000000000..037fa7861 --- /dev/null +++ b/contracts/Mocks/FallbackExtensions/RevertErrorsTestExtension.sol @@ -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(); + } +} diff --git a/contracts/Mocks/LSP17ExtendableTester.sol b/contracts/Mocks/LSP17ExtendableTester.sol new file mode 100644 index 000000000..0cd60031a --- /dev/null +++ b/contracts/Mocks/LSP17ExtendableTester.sol @@ -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]; + } +} diff --git a/package.json b/package.json index 1f9451f64..cf0245c3d 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/tests/LSP17ContractExtension/LSP17Extendable.behaviour.ts b/tests/LSP17ContractExtension/LSP17Extendable.behaviour.ts index 82d25e2de..411ded9ec 100644 --- a/tests/LSP17ContractExtension/LSP17Extendable.behaviour.ts +++ b/tests/LSP17ContractExtension/LSP17Extendable.behaviour.ts @@ -265,7 +265,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { + 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 = @@ -667,6 +667,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { await expect( context.accounts[0].sendTransaction({ @@ -677,7 +678,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { + 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', () => { @@ -694,6 +695,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { it('should pass', async () => { await expect( @@ -705,6 +707,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { describe('with sending value', () => { it('should pass and emit ValueReceived value', async () => { @@ -726,6 +729,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { it('should pass', async () => { const graffiti = @@ -744,6 +748,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { describe('when setting an extension that reverts', () => { let revertFallbackExtension: RevertFallbackExtension; @@ -762,6 +767,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { it('should revert', async () => { await expect( @@ -772,6 +778,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { it('should revert', async () => { const graffiti = @@ -801,7 +808,7 @@ export const shouldBehaveLikeLSP17 = (buildContext: () => Promise { + 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; diff --git a/tests/LSP17ContractExtension/LSP17Extendable.test.ts b/tests/LSP17ContractExtension/LSP17Extendable.test.ts new file mode 100644 index 000000000..130afe615 --- /dev/null +++ b/tests/LSP17ContractExtension/LSP17Extendable.test.ts @@ -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; + }); + }); + }); +});