diff --git a/packages/core/contracts/test/Namespaced.sol b/packages/core/contracts/test/Namespaced.sol index 3e3d9877b..8f33d4f67 100644 --- a/packages/core/contracts/test/Namespaced.sol +++ b/packages/core/contracts/test/Namespaced.sol @@ -24,18 +24,6 @@ contract Example { } } -contract DuplicateNamespace { - /// @custom:storage-location erc7201:conflicting - struct Conflicting1 { - uint256 b; - } - - /// @custom:storage-location erc7201:conflicting - struct Conflicting2 { - uint256 c; - } -} - contract MultipleNamespaces { /// @custom:storage-location erc7201:one struct S1 { diff --git a/packages/core/contracts/test/NamespacedConflicts.sol b/packages/core/contracts/test/NamespacedConflicts.sol new file mode 100644 index 000000000..2cf629e83 --- /dev/null +++ b/packages/core/contracts/test/NamespacedConflicts.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +contract DuplicateNamespace { + /// @custom:storage-location erc7201:conflicting + struct Conflicting1 { + uint256 b; + } + + /// @custom:storage-location erc7201:conflicting + struct Conflicting2 { + uint256 c; + } +} + +contract ConflictsWithParent is DuplicateNamespace { + /// @custom:storage-location erc7201:conflicting + struct Conflicting { + uint256 a; + } +} + +contract ConflictsInBothParents is DuplicateNamespace, ConflictsWithParent { +} \ No newline at end of file diff --git a/packages/core/hardhat.config.js b/packages/core/hardhat.config.js index 81458e660..de9b6e8fb 100644 --- a/packages/core/hardhat.config.js +++ b/packages/core/hardhat.config.js @@ -40,6 +40,7 @@ module.exports = { ], overrides: { 'contracts/test/Namespaced.sol': { version: '0.8.20', settings }, + 'contracts/test/NamespacedConflicts.sol': { version: '0.8.20', settings }, }, }, etherscan: { diff --git a/packages/core/src/storage-namespaced-conflicts.test.ts b/packages/core/src/storage-namespaced-conflicts.test.ts new file mode 100644 index 000000000..44e7cb03b --- /dev/null +++ b/packages/core/src/storage-namespaced-conflicts.test.ts @@ -0,0 +1,49 @@ +import _test, { TestFn } from 'ava'; +import { ContractDefinition } from 'solidity-ast'; +import { findAll, astDereferencer } from 'solidity-ast/utils'; +import { artifacts } from 'hardhat'; + +import { SolcOutput } from './solc-api'; +import { StorageLayout } from './storage/layout'; +import { extractStorageLayout } from './storage/extract'; +import { solcInputOutputDecoder } from './src-decoder'; + +interface Context { + extractStorageLayout: (contract: string) => ReturnType; +} + +const test = _test as TestFn; + +test.before(async t => { + const buildInfo = await artifacts.getBuildInfo('contracts/test/NamespacedConflicts.sol:DuplicateNamespace'); + if (buildInfo === undefined) { + throw new Error('Build info not found'); + } + const solcOutput: SolcOutput = buildInfo.output; + const contracts: Record = {}; + const storageLayouts: Record = {}; + for (const def of findAll('ContractDefinition', solcOutput.sources['contracts/test/NamespacedConflicts.sol'].ast)) { + contracts[def.name] = def; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + storageLayouts[def.name] = solcOutput.contracts['contracts/test/NamespacedConflicts.sol'][def.name].storageLayout!; + } + const deref = astDereferencer(solcOutput); + const decodeSrc = solcInputOutputDecoder(buildInfo.input, solcOutput); + t.context.extractStorageLayout = name => + extractStorageLayout(contracts[name], decodeSrc, deref, storageLayouts[name]); +}); + +test('duplicate namespace', t => { + const error = t.throws(() => t.context.extractStorageLayout('DuplicateNamespace')); + t.snapshot(error?.message); +}); + +test('conflicts with parent', t => { + const error = t.throws(() => t.context.extractStorageLayout('ConflictsWithParent')); + t.snapshot(error?.message); +}); + +test('conflicts in both parents', t => { + const error = t.throws(() => t.context.extractStorageLayout('ConflictsInBothParents')); + t.snapshot(error?.message); +}); \ No newline at end of file diff --git a/packages/core/src/storage-namespaced-conflicts.test.ts.md b/packages/core/src/storage-namespaced-conflicts.test.ts.md new file mode 100644 index 000000000..965c2bd7c --- /dev/null +++ b/packages/core/src/storage-namespaced-conflicts.test.ts.md @@ -0,0 +1,43 @@ +# Snapshot report for `src/storage-namespaced-conflicts.test.ts` + +The actual snapshot is saved in `storage-namespaced-conflicts.test.ts.snap`. + +Generated by [AVA](https://avajs.dev). + +## duplicate namespace + +> Snapshot 1 + + `Namespace erc7201:conflicting is defined multiple times for contract DuplicateNamespace␊ + ␊ + The namespace erc7201:conflicting was found in structs at the following locations:␊ + - contracts/test/NamespacedConflicts.sol:6␊ + - contracts/test/NamespacedConflicts.sol:11␊ + ␊ + Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:' in your contract and its inherited contracts.` + +## conflicts with parent + +> Snapshot 1 + + `Namespace erc7201:conflicting is defined multiple times for contract ConflictsWithParent␊ + ␊ + The namespace erc7201:conflicting was found in structs at the following locations:␊ + - contracts/test/NamespacedConflicts.sol:18␊ + - contracts/test/NamespacedConflicts.sol:6␊ + - contracts/test/NamespacedConflicts.sol:11␊ + ␊ + Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:' in your contract and its inherited contracts.` + +## conflicts in both parents + +> Snapshot 1 + + `Namespace erc7201:conflicting is defined multiple times for contract ConflictsInBothParents␊ + ␊ + The namespace erc7201:conflicting was found in structs at the following locations:␊ + - contracts/test/NamespacedConflicts.sol:18␊ + - contracts/test/NamespacedConflicts.sol:6␊ + - contracts/test/NamespacedConflicts.sol:11␊ + ␊ + Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:' in your contract and its inherited contracts.` diff --git a/packages/core/src/storage-namespaced-conflicts.test.ts.snap b/packages/core/src/storage-namespaced-conflicts.test.ts.snap new file mode 100644 index 000000000..47cac122a Binary files /dev/null and b/packages/core/src/storage-namespaced-conflicts.test.ts.snap differ diff --git a/packages/core/src/storage-namespaced.test.ts b/packages/core/src/storage-namespaced.test.ts index e5475942b..33204b0bc 100644 --- a/packages/core/src/storage-namespaced.test.ts +++ b/packages/core/src/storage-namespaced.test.ts @@ -40,11 +40,6 @@ test('layout', t => { t.snapshot(stabilizeStorageLayout(layout)); }); -test('duplicate namespace', t => { - const layout = t.context.extractStorageLayout('DuplicateNamespace'); - t.snapshot(stabilizeStorageLayout(layout)); -}); - test('multiple namespaces', t => { const layout = t.context.extractStorageLayout('MultipleNamespaces'); t.snapshot(stabilizeStorageLayout(layout)); diff --git a/packages/core/src/storage/namespace.ts b/packages/core/src/storage/namespace.ts index ee25818cc..a7e678807 100644 --- a/packages/core/src/storage/namespace.ts +++ b/packages/core/src/storage/namespace.ts @@ -61,7 +61,7 @@ class DuplicateNamespaceError extends UpgradesError { super( `Namespace ${id} is defined multiple times for contract ${contractName}`, () => `\ -The namespace ${id} was found in: +The namespace ${id} was found in structs at the following locations: - ${srcs.join('\n- ')} Use a unique namespace id for each struct annotated with '@custom:storage-location erc7201:' in your contract and its inherited contracts.`, @@ -111,7 +111,7 @@ function pushInheritedNamespaces( if (namespacedContext === undefined) { for (let i = 0; i < origInheritIds.length; i++) { const origInherit = origContext.deref(['ContractDefinition'], origInheritIds[i]); - pushDirectNamespaces(namespaces, decodeSrc, layout, origContext, origInherit); + pushDirectNamespaces(namespaces, decodeSrc, layout, { ...origContext, contractDef: origInherit }, origInherit); } } else { const namespacedInheritIds = namespacedContext.contractDef.linearizedBaseContracts.slice(1); @@ -215,7 +215,7 @@ function getOriginalStruct(structCanonicalName: string, origContractDef: Contrac } } } - throw new Error(`Could not find original source location for namespace struct with name ${structCanonicalName}`); + throw new Error(`Could not find original source location for namespace struct with name ${structCanonicalName} from contract ${origContractDef.name}`); } function getOriginalMemberSrc(structCanonicalName: string, memberLabel: string, origContractDef: ContractDefinition) {