Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stale-lizards-cheat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`ERC721URIStorage`: Add `_suffixURI`, an internal getter for retrieving the custom tokenURI without the base prefix.
5 changes: 5 additions & 0 deletions .changeset/young-corners-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`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.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### Breaking changes

- `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.

## 5.5.0 (2025-10-31)

### Bug fixes
Expand Down
9 changes: 9 additions & 0 deletions contracts/governance/extensions/GovernorStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ abstract contract GovernorStorage is Governor {
function queue(uint256 proposalId) public virtual {
// here, using storage is more efficient than memory
ProposalDetails storage details = _proposalDetails[proposalId];
if (details.descriptionHash == 0) {
revert GovernorNonexistentProposal(proposalId);
}
queue(details.targets, details.values, details.calldatas, details.descriptionHash);
}

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

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

Expand Down
15 changes: 11 additions & 4 deletions contracts/token/ERC721/extensions/ERC721URIStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
function tokenURI(uint256 tokenId) public view virtual override returns (string memory) {
_requireOwned(tokenId);

string memory _tokenURI = _tokenURIs[tokenId];
string memory base = _baseURI();
string memory suffix = _suffixURI(tokenId);

// If there is no base URI, return the token URI.
if (bytes(base).length == 0) {
return _tokenURI;
return suffix;
}
// If both are set, concatenate the baseURI and tokenURI (via string.concat).
if (bytes(_tokenURI).length > 0) {
return string.concat(base, _tokenURI);
if (bytes(suffix).length > 0) {
return string.concat(base, suffix);
}

return super.tokenURI(tokenId);
Expand All @@ -52,4 +52,11 @@ abstract contract ERC721URIStorage is IERC4906, ERC721 {
_tokenURIs[tokenId] = _tokenURI;
emit MetadataUpdate(tokenId);
}

/**
* @dev Returns the suffix part of the tokenURI for `tokenId`.
*/
function _suffixURI(uint256 tokenId) internal view virtual returns (string memory) {
return _tokenURIs[tokenId];
}
}
24 changes: 19 additions & 5 deletions contracts/utils/RLP.sol
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ library RLP {
}

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

/// @dev Encode a uint256 as RLP.
/**
* @dev Encode an uint256 as an RLP scalar.
*
* Unlike {encode-bytes32-}, this function uses scalar encoding that removes the prefix zeros.
*/
function encode(uint256 input) internal pure returns (bytes memory result) {
if (input < SHORT_OFFSET) {
assembly ("memory-safe") {
Expand All @@ -186,9 +190,19 @@ library RLP {
}
}

/// @dev Encode a bytes32 as RLP. Type alias for {encode-uint256-}.
function encode(bytes32 input) internal pure returns (bytes memory) {
return encode(uint256(input));
/**
* @dev Encode a bytes32 as an RLP item of fixed size (32 bytes).
*
* Unlike {encode-uint256-}, this function uses array encoding that preserves the prefix zeros.
*/
function encode(bytes32 input) internal pure returns (bytes memory result) {
assembly ("memory-safe") {
result := mload(0x40)
mstore(result, 0x21) // length of the encoded data: 1 (prefix) + 0x20
mstore8(add(result, 0x20), 0xa0) // prefix: SHORT_OFFSET + 0x20
mstore(add(result, 0x21), input)
mstore(0x40, add(result, 0x41)) // reserve memory
}
}

/// @dev Encode a bytes buffer as RLP.
Expand Down
4 changes: 2 additions & 2 deletions docs/modules/ROOT/pages/governance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ This governance protocol is generally implemented in a special-purpose contract

OpenZeppelin’s Governor system was designed with a concern for compatibility with existing systems that were based on Compound’s GovernorAlpha and GovernorBravo. Because of this, you will find that many modules are presented in two variants, one of which is built for compatibility with those systems.

=== ERC20Votes & ERC20VotesComp
=== ERC20Votes

The ERC-20 extension to keep track of votes and vote delegation is one such case. The shorter one is the more generic version because it can support token supplies greater than 2^96, while the “Comp” variant is limited in that regard, but exactly fits the interface of the COMP token that is used by GovernorAlpha and Bravo. Both contract variants share the same events, so they are fully compatible when looking at events only.
The ERC-20 extension to keep track of votes and vote delegation is `ERC20Votes`. This extension supports token supplies greater than 2^96 and keeps a history of voting power that can be delegated and queried at past timepoints.

=== Governor & GovernorStorage

Expand Down
24 changes: 20 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 20 additions & 0 deletions test/governance/extensions/GovernorStorage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,26 @@ describe('GovernorStorage', function () {
.to.emit(this.mock, 'ProposalCanceled')
.withArgs(this.proposal.id);
});

describe('with non-existing proposalId', function () {
it('queue', async function () {
await expect(this.mock.queue(this.proposal.id))
.to.be.revertedWithCustomError(this.mock, 'GovernorNonexistentProposal')
.withArgs(this.proposal.id);
});

it('execute', async function () {
await expect(this.mock.execute(this.proposal.id))
.to.be.revertedWithCustomError(this.mock, 'GovernorNonexistentProposal')
.withArgs(this.proposal.id);
});

it('cancel', async function () {
await expect(this.mock.cancel(this.proposal.id))
.to.be.revertedWithCustomError(this.mock, 'GovernorNonexistentProposal')
.withArgs(this.proposal.id);
});
});
});
}
});
29 changes: 21 additions & 8 deletions test/token/ERC721/extensions/ERC721URIStorage.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,22 @@ describe('ERC721URIStorage', function () {
});

it('it is empty by default', async function () {
expect(await this.token.tokenURI(tokenId)).to.equal('');
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal('');
await expect(this.token.tokenURI(tokenId)).to.eventually.equal('');
});

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

it('can be set for a token id', async function () {
await this.token.$_setTokenURI(tokenId, sampleUri);
expect(await this.token.tokenURI(tokenId)).to.equal(sampleUri);

await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(sampleUri);
});

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

// value will be accessible after mint
await this.token.$_mint(this.owner, nonExistentTokenId);
expect(await this.token.tokenURI(nonExistentTokenId)).to.equal(sampleUri);
await expect(this.token.tokenURI(nonExistentTokenId)).to.eventually.equal(sampleUri);
});

it('base URI can be set', async function () {
await this.token.setBaseURI(baseURI);
expect(await this.token.$_baseURI()).to.equal(baseURI);
await expect(this.token.$_baseURI()).to.eventually.equal(baseURI);
});

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

expect(await this.token.tokenURI(tokenId)).to.equal(baseURI + sampleUri);
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(baseURI + sampleUri);
});

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

await this.token.setBaseURI(otherBaseURI);
expect(await this.token.tokenURI(tokenId)).to.equal(otherBaseURI + sampleUri);

await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(otherBaseURI + sampleUri);
});

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

expect(await this.token.tokenURI(tokenId)).to.equal(baseURI + tokenId);
await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal('');
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(baseURI + tokenId);
});

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

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

await this.token.$_burn(tokenId);

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

await this.token.$_burn(tokenId);

await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
await expect(this.token.tokenURI(tokenId))
.to.be.revertedWithCustomError(this.token, 'ERC721NonexistentToken')
.withArgs(tokenId);

await this.token.$_mint(this.owner, tokenId);
expect(await this.token.tokenURI(tokenId)).to.equal(sampleUri);

await expect(this.token.$_suffixURI(tokenId)).to.eventually.equal(sampleUri);
await expect(this.token.tokenURI(tokenId)).to.eventually.equal(sampleUri);
});
});
});
37 changes: 17 additions & 20 deletions test/utils/RLP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,36 +91,33 @@ describe('RLP', function () {
});

it('encode/decode bytes32', async function () {
for (const { input, expected } of [
{ input: '0x0000000000000000000000000000000000000000000000000000000000000000', expected: '0x80' },
{ input: '0x0000000000000000000000000000000000000000000000000000000000000001', expected: '0x01' },
{
input: '0x1000000000000000000000000000000000000000000000000000000000000000',
expected: '0xa01000000000000000000000000000000000000000000000000000000000000000',
},
for (const input of [
'0x0000000000000000000000000000000000000000000000000000000000000000',
'0x0000000000000000000000000000000000000000000000000000000000000001',
'0x1000000000000000000000000000000000000000000000000000000000000000',
generators.bytes32(),
]) {
await expect(this.mock.$encode_bytes32(input)).to.eventually.equal(expected);
await expect(this.mock.$decodeBytes32(expected)).to.eventually.equal(input);
const encoded = ethers.encodeRlp(input);
await expect(this.mock.$encode_bytes32(input)).to.eventually.equal(encoded);
await expect(this.mock.$decodeBytes32(encoded)).to.eventually.equal(input);
}

// Compact encoding for 1234
await expect(this.mock.$decodeBytes32('0x8204d2')).to.eventually.equal(
'0x00000000000000000000000000000000000000000000000000000000000004d2',
); // Canonical encoding for 1234
);
// Encoding with one leading zero
await expect(this.mock.$decodeBytes32('0x830004d2')).to.eventually.equal(
'0x00000000000000000000000000000000000000000000000000000000000004d2',
); // Non-canonical encoding with leading zero
);
// Encoding with two leading zeros
await expect(this.mock.$decodeBytes32('0x84000004d2')).to.eventually.equal(
'0x00000000000000000000000000000000000000000000000000000000000004d2',
); // Non-canonical encoding with two leading zeros

// Canonical encoding for zero and non-canonical encodings with leading zeros
await expect(this.mock.$decodeBytes32('0x80')).to.eventually.equal(
'0x0000000000000000000000000000000000000000000000000000000000000000',
);
// 1 leading zero is not allowed for single bytes less than 0x80, they must be encoded as themselves
await expect(this.mock.$decodeBytes32('0x820000')).to.eventually.equal(
'0x0000000000000000000000000000000000000000000000000000000000000000',
); // Non-canonical encoding with two leading zeros
// Encoding for the value
await expect(this.mock.$decodeBytes32('0x80')).to.eventually.equal(ethers.ZeroHash);
// Encoding for two zeros (and nothing else)
await expect(this.mock.$decodeBytes32('0x820000')).to.eventually.equal(ethers.ZeroHash);
});

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