Skip to content

Commit a7e6cb5

Browse files
ekpyronekpyron
authored
Merge pull request #15528 from ethereum/storageLocationsTypeChecking
Type checking for contract explicit storage base location
2 parents eb21671 + 7ef0ef8 commit a7e6cb5

File tree

89 files changed

+530
-25
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

89 files changed

+530
-25
lines changed

‎libsolidity/analysis/ContractLevelChecker.cpp‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,41 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract)
9797
checkBaseABICompatibility(_contract);
9898
checkPayableFallbackWithoutReceive(_contract);
9999
checkStorageSize(_contract);
100+
checkStorageLayoutSpecifier(_contract);
100101

101102
return !Error::containsErrors(m_errorReporter.errors());
102103
}
103104

105+
void ContractLevelChecker::checkStorageLayoutSpecifier(ContractDefinition const& _contract)
106+
{
107+
if (_contract.storageLayoutSpecifier())
108+
{
109+
solAssert(!_contract.isLibrary() && !_contract.isInterface());
110+
111+
if (_contract.abstract())
112+
m_errorReporter.typeError(
113+
7587_error,
114+
_contract.storageLayoutSpecifier()->location(),
115+
"Storage layout cannot be specified for abstract contracts."
116+
);
117+
}
118+
119+
for (auto const* ancestorContract: _contract.annotation().linearizedBaseContracts | ranges::views::reverse)
120+
{
121+
if (*ancestorContract == _contract)
122+
continue;
123+
if (ancestorContract->storageLayoutSpecifier())
124+
m_errorReporter.typeError(
125+
8894_error,
126+
_contract.location(),
127+
SecondarySourceLocation().append(
128+
"Storage layout was already specified here.",
129+
ancestorContract->storageLayoutSpecifier()->location()
130+
),
131+
"Storage layout can only be specified in the most derived contract."
132+
);
133+
}
134+
}
104135
void ContractLevelChecker::checkDuplicateFunctions(ContractDefinition const& _contract)
105136
{
106137
/// Checks that two functions with the same name defined in this contract have different

‎libsolidity/analysis/ContractLevelChecker.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ class ContractLevelChecker
9090
void checkPayableFallbackWithoutReceive(ContractDefinition const& _contract);
9191
/// Error if the contract requires too much storage
9292
void checkStorageSize(ContractDefinition const& _contract);
93+
/// Checks if the storage layout specifier is properly assigned in the inheritance tree and not applied to an abstract contract
94+
void checkStorageLayoutSpecifier(ContractDefinition const& _contract);
9395

9496
OverrideChecker m_overrideChecker;
9597
langutil::ErrorReporter& m_errorReporter;

‎libsolidity/analysis/PostTypeContractLevelChecker.cpp‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,19 @@
2222

2323
#include <libsolidity/analysis/PostTypeContractLevelChecker.h>
2424

25+
#include <fmt/format.h>
2526
#include <libsolidity/ast/AST.h>
27+
#include <libsolidity/ast/ASTUtils.h>
28+
#include <libsolidity/ast/TypeProvider.h>
2629
#include <libsolutil/FunctionSelector.h>
2730
#include <liblangutil/ErrorReporter.h>
2831

32+
#include <limits>
33+
2934
using namespace solidity;
3035
using namespace solidity::langutil;
3136
using namespace solidity::frontend;
37+
using namespace solidity::util;
3238

3339
bool PostTypeContractLevelChecker::check(SourceUnit const& _sourceUnit)
3440
{
@@ -51,7 +57,7 @@ bool PostTypeContractLevelChecker::check(ContractDefinition const& _contract)
5157
for (ErrorDefinition const* error: _contract.interfaceErrors())
5258
{
5359
std::string signature = error->functionType(true)->externalSignature();
54-
uint32_t hash = util::selectorFromSignatureU32(signature);
60+
uint32_t hash = selectorFromSignatureU32(signature);
5561
// Fail if there is a different signature for the same hash.
5662
if (!errorHashes[hash].empty() && !errorHashes[hash].count(signature))
5763
{
@@ -67,5 +73,66 @@ bool PostTypeContractLevelChecker::check(ContractDefinition const& _contract)
6773
errorHashes[hash][signature] = error->location();
6874
}
6975

76+
if (auto const* layoutSpecifier = _contract.storageLayoutSpecifier())
77+
checkStorageLayoutSpecifier(*layoutSpecifier);
78+
7079
return !Error::containsErrors(m_errorReporter.errors());
7180
}
81+
82+
void PostTypeContractLevelChecker::checkStorageLayoutSpecifier(StorageLayoutSpecifier const& _storageLayoutSpecifier)
83+
{
84+
Expression const& baseSlotExpression = _storageLayoutSpecifier.baseSlotExpression();
85+
86+
if (!*baseSlotExpression.annotation().isPure)
87+
{
88+
// TODO: introduce and handle erc7201 as a builtin function
89+
m_errorReporter.typeError(
90+
1139_error,
91+
baseSlotExpression.location(),
92+
"The base slot of the storage layout must be a compile-time constant expression."
93+
);
94+
return;
95+
}
96+
97+
auto const* baseSlotExpressionType = type(baseSlotExpression);
98+
auto const* rationalType = dynamic_cast<RationalNumberType const*>(baseSlotExpressionType);
99+
if (!rationalType)
100+
{
101+
m_errorReporter.typeError(
102+
6396_error,
103+
baseSlotExpression.location(),
104+
"The base slot of the storage layout must evaluate to a rational number."
105+
);
106+
return;
107+
}
108+
109+
if (rationalType->isFractional())
110+
{
111+
m_errorReporter.typeError(
112+
1763_error,
113+
baseSlotExpression.location(),
114+
"The base slot of the storage layout must evaluate to an integer."
115+
);
116+
return;
117+
}
118+
solAssert(rationalType->value().denominator() == 1);
119+
120+
if (
121+
rationalType->value().numerator() < 0 ||
122+
rationalType->value().numerator() > std::numeric_limits<u256>::max()
123+
)
124+
{
125+
m_errorReporter.typeError(
126+
6753_error,
127+
baseSlotExpression.location(),
128+
fmt::format(
129+
"The base slot of the storage layout evaluates to {}, which is outside the range of type uint256.",
130+
formatNumberReadable(rationalType->value().numerator())
131+
)
132+
);
133+
return;
134+
}
135+
136+
solAssert(baseSlotExpressionType->isImplicitlyConvertibleTo(*TypeProvider::uint256()));
137+
_storageLayoutSpecifier.annotation().baseSlot = u256(rationalType->value().numerator());
138+
}

‎libsolidity/analysis/PostTypeContractLevelChecker.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ class PostTypeContractLevelChecker
5050
/// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings
5151
bool check(ContractDefinition const& _contract);
5252

53+
void checkStorageLayoutSpecifier(StorageLayoutSpecifier const& _storageLayoutSpecifier);
54+
5355
langutil::ErrorReporter& m_errorReporter;
5456
};
5557

‎libsolidity/ast/AST.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,11 @@ StorageLayoutSpecifier::StorageLayoutSpecifier(
383383
solAssert(_location.contains(m_baseSlotExpression->location()));
384384
}
385385

386+
StorageLayoutSpecifierAnnotation& StorageLayoutSpecifier::annotation() const
387+
{
388+
return initAnnotation<StorageLayoutSpecifierAnnotation>();
389+
}
390+
386391
TypeNameAnnotation& TypeName::annotation() const
387392
{
388393
return initAnnotation<TypeNameAnnotation>();

‎libsolidity/ast/AST.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,8 @@ class StorageLayoutSpecifier : public ASTNode
622622
void accept(ASTConstVisitor& _visitor) const override;
623623

624624
Expression const& baseSlotExpression() const { solAssert(m_baseSlotExpression); return *m_baseSlotExpression; }
625+
StorageLayoutSpecifierAnnotation& annotation() const override;
626+
625627
private:
626628
ASTPointer<Expression> m_baseSlotExpression;
627629
};

‎libsolidity/ast/ASTAnnotations.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <libsolidity/ast/ASTEnums.h>
2828
#include <libsolidity/ast/ExperimentalFeatures.h>
2929

30+
#include <libsolutil/Numeric.h>
3031
#include <libsolutil/SetOnce.h>
3132

3233
#include <map>
@@ -173,6 +174,12 @@ struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, StructurallyDocu
173174
std::map<FunctionDefinition const*, uint64_t> internalFunctionIDs;
174175
};
175176

177+
struct StorageLayoutSpecifierAnnotation: ASTAnnotation
178+
{
179+
// The evaluated value of the expression specifying the contract storage layout base
180+
util::SetOnce<u256> baseSlot;
181+
};
182+
176183
struct CallableDeclarationAnnotation: DeclarationAnnotation
177184
{
178185
/// The set of functions/modifiers/events this callable overrides.

‎test/cmdlineTests/storage_layout_already_defined_in_ancestor/args‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-

‎test/cmdlineTests/storage_layout_already_defined_in_ancestor/err‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
Error: Storage layout can only be specified in the most derived contract.
2+
--> <stdin>:5:1:
3+
|
4+
5 | contract B is A {}
5+
| ^^^^^^^^^^^^^^^^^^
6+
Note: Storage layout was already specified here.
7+
--> <stdin>:4:12:
8+
|
9+
4 | contract A layout at 0x1234 {}
10+
| ^^^^^^^^^^^^^^^^
11+
12+
Error: Storage layout can only be specified in the most derived contract.
13+
--> <stdin>:6:1:
14+
|
15+
6 | contract C layout at 0x1234 is B {}
16+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
Note: Storage layout was already specified here.
18+
--> <stdin>:4:12:
19+
|
20+
4 | contract A layout at 0x1234 {}
21+
| ^^^^^^^^^^^^^^^^

‎test/cmdlineTests/storage_layout_already_defined_in_ancestor/exit‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
1

0 commit comments

Comments
 (0)