Skip to content

Commit ce5e6ed

Browse files
sashass1315Amxx
andauthored
Revert with supplied proposalId for unknown proposals in GovernorStorage (#6160)
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
1 parent 829492c commit ce5e6ed

File tree

2 files changed

+29
-0
lines changed

2 files changed

+29
-0
lines changed

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

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
});

0 commit comments

Comments
 (0)