Skip to content

Commit 9b1585a

Browse files
AmxxAmxx
authored
Merge branch 'master' into typo-fixes
2 parents 8591fa0 + ce5e6ed commit 9b1585a

File tree

10 files changed

+131
-41
lines changed

10 files changed

+131
-41
lines changed

‎.changeset/stale-lizards-cheat.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`ERC721URIStorage`: Add `_suffixURI`, an internal getter for retrieving the custom tokenURI without the base prefix.

‎.changeset/young-corners-help.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': minor
3+
---
4+
5+
`RLP`: Encode `bytes32` as a fixed size item and not as a scalar in `encode(bytes32)`. Scalar RLP encoding remains available by casting to a `uint256` and using the `encode(uint256)` function.

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
### Breaking changes
4+
5+
- `RLP`: The `encode(bytes32)` function now encodes `bytes32` as a fixed size item and not as a scalar in `encode(uint256)`. Users must replace calls to `encode(bytes32)` with `encode(uint256(bytes32))` to preserve the same behavior.
6+
37
## 5.5.0 (2025-10-31)
48

59
### Bug fixes

‎contracts/governance/extensions/GovernorStorage.sol‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ abstract contract GovernorStorage is Governor {
5555
function queue(uint256 proposalId) public virtual {
5656
// here, using storage is more efficient than memory
5757
ProposalDetails storage details = _proposalDetails[proposalId];
58+
if (details.descriptionHash == 0) {
59+
revert GovernorNonexistentProposal(proposalId);
60+
}
5861
queue(details.targets, details.values, details.calldatas, details.descriptionHash);
5962
}
6063

@@ -64,6 +67,9 @@ abstract contract GovernorStorage is Governor {
6467
function execute(uint256 proposalId) public payable virtual {
6568
// here, using storage is more efficient than memory
6669
ProposalDetails storage details = _proposalDetails[proposalId];
70+
if (details.descriptionHash == 0) {
71+
revert GovernorNonexistentProposal(proposalId);
72+
}
6773
execute(details.targets, details.values, details.calldatas, details.descriptionHash);
6874
}
6975

@@ -73,6 +79,9 @@ abstract contract GovernorStorage is Governor {
7379
function cancel(uint256 proposalId) public virtual {
7480
// here, using storage is more efficient than memory
7581
ProposalDetails storage details = _proposalDetails[proposalId];
82+
if (details.descriptionHash == 0) {
83+
revert GovernorNonexistentProposal(proposalId);
84+
}
7685
cancel(details.targets, details.values, details.calldatas, details.descriptionHash);
7786
}
7887

‎contracts/token/ERC721/extensions/ERC721URIStorage.sol‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
2828
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
2929
_requireOwned(tokenId);
3030

31-
string memory _tokenURI = _tokenURIs[tokenId];
3231
string memory base = _baseURI();
32+
string memory suffix = _suffixURI(tokenId);
3333

3434
// If there is no base URI, return the token URI.
3535
if (bytes(base).length == 0) {
36-
return _tokenURI;
36+
return suffix;
3737
}
3838
// If both are set, concatenate the baseURI and tokenURI (via string.concat).
39-
if (bytes(_tokenURI).length > 0) {
40-
return string.concat(base, _tokenURI);
39+
if (bytes(suffix).length > 0) {
40+
return string.concat(base, suffix);
4141
}
4242

4343
return super.tokenURI(tokenId);
@@ -52,4 +52,11 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
5252
_tokenURIs[tokenId] = _tokenURI;
5353
emit MetadataUpdate(tokenId);
5454
}
55+
56+
/**
57+
* @dev Returns the suffix part of the tokenURI for `tokenId`.
58+
*/
59+
function _suffixURI(uint256 tokenId) internal view virtual returns (string memory) {
60+
return _tokenURIs[tokenId];
61+
}
5562
}

‎contracts/utils/RLP.sol‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ library RLP {
151151
}
152152

153153
/**
154-
* @dev Encode an address as RLP.
154+
* @dev Encode an address as an RLP item of fixed size (20 bytes).
155155
*
156156
* The address is encoded with its leading zeros (if it has any). If someone wants to encode the address as a scalar,
157157
* they can cast it to an uint256 and then call the corresponding {encode} function.
@@ -165,7 +165,11 @@ library RLP {
165165
}
166166
}
167167

168-
/// @dev Encode a uint256 as RLP.
168+
/**
169+
* @dev Encode an uint256 as an RLP scalar.
170+
*
171+
* Unlike {encode-bytes32-}, this function uses scalar encoding that removes the prefix zeros.
172+
*/
169173
function encode(uint256 input) internal pure returns (bytes memory result) {
170174
if (input < SHORT_OFFSET) {
171175
assembly ("memory-safe") {
@@ -186,9 +190,19 @@ library RLP {
186190
}
187191
}
188192

189-
/// @dev Encode a bytes32 as RLP. Type alias for {encode-uint256-}.
190-
function encode(bytes32 input) internal pure returns (bytes memory) {
191-
return encode(uint256(input));
193+
/**
194+
* @dev Encode a bytes32 as an RLP item of fixed size (32 bytes).
195+
*
196+
* Unlike {encode-uint256-}, this function uses array encoding that preserves the prefix zeros.
197+
*/
198+
function encode(bytes32 input) internal pure returns (bytes memory result) {
199+
assembly ("memory-safe") {
200+
result := mload(0x40)
201+
mstore(result, 0x21) // length of the encoded data: 1 (prefix) + 0x20
202+
mstore8(add(result, 0x20), 0xa0) // prefix: SHORT_OFFSET + 0x20
203+
mstore(add(result, 0x21), input)
204+
mstore(0x40, add(result, 0x41)) // reserve memory
205+
}
192206
}
193207

194208
/// @dev Encode a bytes buffer as RLP.

‎package-lock.json‎

Lines changed: 20 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/governance/extensions/GovernorStorage.test.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,26 @@ describe('GovernorStorage', function () {
150150
.to.emit(this.mock, 'ProposalCanceled')
151151
.withArgs(this.proposal.id);
152152
});
153+
154+
describe('with non-existing proposalId', function () {
155+
it('queue', async function () {
156+
await expect(this.mock.queue(this.proposal.id))
157+
.to.be.revertedWithCustomError(this.mock, 'GovernorNonexistentProposal')
158+
.withArgs(this.proposal.id);
159+
});
160+
161+
it('execute', async function () {
162+
await expect(this.mock.execute(this.proposal.id))
163+
.to.be.revertedWithCustomError(this.mock, 'GovernorNonexistentProposal')
164+
.withArgs(this.proposal.id);
165+
});
166+
167+
it('cancel', async function () {
168+
await expect(this.mock.cancel(this.proposal.id))
169+
.to.be.revertedWithCustomError(this.mock, 'GovernorNonexistentProposal')
170+
.withArgs(this.proposal.id);
171+
});
172+
});
153173
});
154174
}
155175
});

‎test/token/ERC721/extensions/ERC721URIStorage.test.js‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,18 +31,22 @@ describe('ERC721URIStorage', function () {
3131
});
3232

3333
it('it is empty by default', async function () {
34-
expect(await this.token.tokenURI(tokenId)).to.equal('');
34+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal('');
35+
await expect(this.token.tokenURI(tokenId)).to.eventually.equal('');
3536
});
3637

3738
it('reverts when queried for non existent token id', async function () {
39+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal('');
3840
await expect(this.token.tokenURI(nonExistentTokenId))
3941
.to.be.revertedWithCustomError(this.token, 'ERC721NonexistentToken')
4042
.withArgs(nonExistentTokenId);
4143
});
4244

4345
it('can be set for a token id', async function () {
4446
await this.token.$_setTokenURI(tokenId, sampleUri);
45-
expect(await this.token.tokenURI(tokenId)).to.equal(sampleUri);
47+
48+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
49+
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(sampleUri);
4650
});
4751

4852
it('setting the uri emits an event', async function () {
@@ -58,38 +62,43 @@ describe('ERC721URIStorage', function () {
5862

5963
// value will be accessible after mint
6064
await this.token.$_mint(this.owner, nonExistentTokenId);
61-
expect(await this.token.tokenURI(nonExistentTokenId)).to.equal(sampleUri);
65+
await expect(this.token.tokenURI(nonExistentTokenId)).to.eventually.equal(sampleUri);
6266
});
6367

6468
it('base URI can be set', async function () {
6569
await this.token.setBaseURI(baseURI);
66-
expect(await this.token.$_baseURI()).to.equal(baseURI);
70+
await expect(this.token.$_baseURI()).to.eventually.equal(baseURI);
6771
});
6872

6973
it('base URI is added as a prefix to the token URI', async function () {
7074
await this.token.setBaseURI(baseURI);
7175
await this.token.$_setTokenURI(tokenId, sampleUri);
7276

73-
expect(await this.token.tokenURI(tokenId)).to.equal(baseURI + sampleUri);
77+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
78+
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(baseURI + sampleUri);
7479
});
7580

7681
it('token URI can be changed by changing the base URI', async function () {
7782
await this.token.setBaseURI(baseURI);
7883
await this.token.$_setTokenURI(tokenId, sampleUri);
7984

8085
await this.token.setBaseURI(otherBaseURI);
81-
expect(await this.token.tokenURI(tokenId)).to.equal(otherBaseURI + sampleUri);
86+
87+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
88+
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(otherBaseURI + sampleUri);
8289
});
8390

8491
it('tokenId is appended to base URI for tokens with no URI', async function () {
8592
await this.token.setBaseURI(baseURI);
8693

87-
expect(await this.token.tokenURI(tokenId)).to.equal(baseURI + tokenId);
94+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal('');
95+
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(baseURI + tokenId);
8896
});
8997

9098
it('tokens without URI can be burnt ', async function () {
9199
await this.token.$_burn(tokenId);
92100

101+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal('');
93102
await expect(this.token.tokenURI(tokenId))
94103
.to.be.revertedWithCustomError(this.token, 'ERC721NonexistentToken')
95104
.withArgs(tokenId);
@@ -100,6 +109,7 @@ describe('ERC721URIStorage', function () {
100109

101110
await this.token.$_burn(tokenId);
102111

112+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
103113
await expect(this.token.tokenURI(tokenId))
104114
.to.be.revertedWithCustomError(this.token, 'ERC721NonexistentToken')
105115
.withArgs(tokenId);
@@ -110,12 +120,15 @@ describe('ERC721URIStorage', function () {
110120

111121
await this.token.$_burn(tokenId);
112122

123+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
113124
await expect(this.token.tokenURI(tokenId))
114125
.to.be.revertedWithCustomError(this.token, 'ERC721NonexistentToken')
115126
.withArgs(tokenId);
116127

117128
await this.token.$_mint(this.owner, tokenId);
118-
expect(await this.token.tokenURI(tokenId)).to.equal(sampleUri);
129+
130+
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
131+
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(sampleUri);
119132
});
120133
});
121134
});

‎test/utils/RLP.test.js‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -91,36 +91,33 @@ describe('RLP', function () {
9191
});
9292

9393
it('encode/decode bytes32', async function () {
94-
for (const { input, expected } of [
95-
{ input: '0x0000000000000000000000000000000000000000000000000000000000000000', expected: '0x80' },
96-
{ input: '0x0000000000000000000000000000000000000000000000000000000000000001', expected: '0x01' },
97-
{
98-
input: '0x1000000000000000000000000000000000000000000000000000000000000000',
99-
expected: '0xa01000000000000000000000000000000000000000000000000000000000000000',
100-
},
94+
for (const input of [
95+
'0x0000000000000000000000000000000000000000000000000000000000000000',
96+
'0x0000000000000000000000000000000000000000000000000000000000000001',
97+
'0x1000000000000000000000000000000000000000000000000000000000000000',
98+
generators.bytes32(),
10199
]) {
102-
await expect(this.mock.$encode_bytes32(input)).to.eventually.equal(expected);
103-
await expect(this.mock.$decodeBytes32(expected)).to.eventually.equal(input);
100+
const encoded = ethers.encodeRlp(input);
101+
await expect(this.mock.$encode_bytes32(input)).to.eventually.equal(encoded);
102+
await expect(this.mock.$decodeBytes32(encoded)).to.eventually.equal(input);
104103
}
105104

105+
// Compact encoding for 1234
106106
await expect(this.mock.$decodeBytes32('0x8204d2')).to.eventually.equal(
107107
'0x00000000000000000000000000000000000000000000000000000000000004d2',
108-
); // Canonical encoding for 1234
108+
);
109+
// Encoding with one leading zero
109110
await expect(this.mock.$decodeBytes32('0x830004d2')).to.eventually.equal(
110111
'0x00000000000000000000000000000000000000000000000000000000000004d2',
111-
); // Non-canonical encoding with leading zero
112+
);
113+
// Encoding with two leading zeros
112114
await expect(this.mock.$decodeBytes32('0x84000004d2')).to.eventually.equal(
113115
'0x00000000000000000000000000000000000000000000000000000000000004d2',
114-
); // Non-canonical encoding with two leading zeros
115-
116-
// Canonical encoding for zero and non-canonical encodings with leading zeros
117-
await expect(this.mock.$decodeBytes32('0x80')).to.eventually.equal(
118-
'0x0000000000000000000000000000000000000000000000000000000000000000',
119116
);
120-
// 1 leading zero is not allowed for single bytes less than 0x80, they must be encoded as themselves
121-
await expect(this.mock.$decodeBytes32('0x820000')).to.eventually.equal(
122-
'0x0000000000000000000000000000000000000000000000000000000000000000',
123-
); // Non-canonical encoding with two leading zeros
117+
// Encoding for the value
118+
await expect(this.mock.$decodeBytes32('0x80')).to.eventually.equal(ethers.ZeroHash);
119+
// Encoding for two zeros (and nothing else)
120+
await expect(this.mock.$decodeBytes32('0x820000')).to.eventually.equal(ethers.ZeroHash);
124121
});
125122

126123
it('encode/decode empty byte', async function () {

0 commit comments

Comments
 (0)