Skip to content
This repository was archived by the owner on Mar 11, 2025. It is now read-only.

Commit 4c0bc4c

Browse files
SebastianBorSebastianBor
authored
Governance V2 cleanup (#2855)
* chore: use exhaustive check for governance account type * fix: realm v1 deserialisation * chore: use exhaustive check for multiple account types * chore: use exhaustive check for seeds * chore: update comments
1 parent ab05e4e commit 4c0bc4c

File tree

8 files changed

+123
-51
lines changed

8 files changed

+123
-51
lines changed

‎Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎governance/chat/program/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "spl-governance-chat"
3-
version = "0.2.1"
3+
version = "0.2.2"
44
description = "Solana Program Library Governance Chat Program"
55
authors = ["Solana Maintainers <[email protected]>"]
66
repository = "https://github.com/solana-labs/solana-program-library"

‎governance/chat/program/src/state.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,15 @@ impl AccountMaxSize for ChatMessage {
6262
}
6363
}
6464

65-
/// Checks whether realm account exists, is initialized and owned by Governance program
65+
/// Checks whether Chat account exists, is initialized and owned by governance-chat program
6666
pub fn assert_is_valid_chat_message(
6767
program_id: &Pubkey,
6868
chat_message_info: &AccountInfo,
6969
) -> Result<(), ProgramError> {
7070
assert_is_valid_account_of_type(
71+
program_id,
7172
chat_message_info,
7273
GovernanceChatAccountType::ChatMessage,
73-
program_id,
7474
)
7575
}
7676

‎governance/program/src/instruction.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,10 +272,10 @@ pub enum GovernanceInstruction {
272272
/// 0. `[writable]` Realm account
273273
/// 1. `[writable]` Governance account
274274
/// 2. `[writable]` Proposal account
275-
/// 4. `[signer]` Signatory account signing off the Proposal
275+
/// 3. `[signer]` Signatory account signing off the Proposal
276276
/// Or Proposal owner if the owner hasn't appointed any signatories
277-
/// 5. `[]` Optional TokenOwnerRecord for the Proposal owner. Required when the owner signs off the Proposal
278-
/// 3. `[writable]` Optional Signatory Record account. Required when non owner sings off the Proposal
277+
/// 4. `[]` TokenOwnerRecord for the Proposal owner, required when the owner signs off the Proposal
278+
/// Or `[writable]` SignatoryRecord account, required when non owner sings off the Proposal
279279
SignOffProposal,
280280

281281
/// Uses your voter weight (deposited Community or Council tokens) to cast a vote on a Proposal

‎governance/program/src/state/governance.rs

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,38 @@ pub struct GovernanceV2 {
8888

8989
impl AccountMaxSize for GovernanceV2 {}
9090

91-
/// Checks if the given account type is one of the Governance account types
91+
/// Checks if the given account type is one of the Governance V2 account types
9292
pub fn is_governance_v2_account_type(account_type: &GovernanceAccountType) -> bool {
93-
*account_type == GovernanceAccountType::GovernanceV2
94-
|| *account_type == GovernanceAccountType::ProgramGovernanceV2
95-
|| *account_type == GovernanceAccountType::MintGovernanceV2
96-
|| *account_type == GovernanceAccountType::TokenGovernanceV2
93+
match account_type {
94+
GovernanceAccountType::GovernanceV2
95+
| GovernanceAccountType::ProgramGovernanceV2
96+
| GovernanceAccountType::MintGovernanceV2
97+
| GovernanceAccountType::TokenGovernanceV2 => true,
98+
GovernanceAccountType::Uninitialized
99+
| GovernanceAccountType::RealmV1
100+
| GovernanceAccountType::RealmV2
101+
| GovernanceAccountType::RealmConfig
102+
| GovernanceAccountType::TokenOwnerRecordV1
103+
| GovernanceAccountType::TokenOwnerRecordV2
104+
| GovernanceAccountType::GovernanceV1
105+
| GovernanceAccountType::ProgramGovernanceV1
106+
| GovernanceAccountType::MintGovernanceV1
107+
| GovernanceAccountType::TokenGovernanceV1
108+
| GovernanceAccountType::ProposalV1
109+
| GovernanceAccountType::ProposalV2
110+
| GovernanceAccountType::SignatoryRecordV1
111+
| GovernanceAccountType::SignatoryRecordV2
112+
| GovernanceAccountType::ProposalInstructionV1
113+
| GovernanceAccountType::ProposalTransactionV2
114+
| GovernanceAccountType::VoteRecordV1
115+
| GovernanceAccountType::VoteRecordV2
116+
| GovernanceAccountType::ProgramMetadata => false,
117+
}
118+
}
119+
120+
/// Checks if the given account type is on of the Governance account types of any version
121+
pub fn is_governance_account_type(account_type: &GovernanceAccountType) -> bool {
122+
is_governance_v1_account_type(account_type) || is_governance_v2_account_type(account_type)
97123
}
98124

99125
impl IsInitialized for GovernanceV2 {
@@ -119,7 +145,23 @@ impl GovernanceV2 {
119145
GovernanceAccountType::TokenGovernanceV1 | GovernanceAccountType::TokenGovernanceV2 => {
120146
get_token_governance_address_seeds(&self.realm, &self.governed_account)
121147
}
122-
_ => return Err(GovernanceToolsError::InvalidAccountType.into()),
148+
GovernanceAccountType::Uninitialized
149+
| GovernanceAccountType::RealmV1
150+
| GovernanceAccountType::TokenOwnerRecordV1
151+
| GovernanceAccountType::ProposalV1
152+
| GovernanceAccountType::SignatoryRecordV1
153+
| GovernanceAccountType::VoteRecordV1
154+
| GovernanceAccountType::ProposalInstructionV1
155+
| GovernanceAccountType::RealmConfig
156+
| GovernanceAccountType::VoteRecordV2
157+
| GovernanceAccountType::ProposalTransactionV2
158+
| GovernanceAccountType::ProposalV2
159+
| GovernanceAccountType::ProgramMetadata
160+
| GovernanceAccountType::RealmV2
161+
| GovernanceAccountType::TokenOwnerRecordV2
162+
| GovernanceAccountType::SignatoryRecordV2 => {
163+
return Err(GovernanceToolsError::InvalidAccountType.into())
164+
}
123165
};
124166

125167
Ok(seeds)
@@ -310,25 +352,12 @@ pub fn get_governance_address<'a>(
310352
.0
311353
}
312354

313-
/// Checks whether governance account exists, is initialized and owned by the Governance program
355+
/// Checks whether the Governance account exists, is initialized and owned by the Governance program
314356
pub fn assert_is_valid_governance(
315357
program_id: &Pubkey,
316358
governance_info: &AccountInfo,
317359
) -> Result<(), ProgramError> {
318-
assert_is_valid_account_of_types(
319-
governance_info,
320-
&[
321-
GovernanceAccountType::GovernanceV1,
322-
GovernanceAccountType::GovernanceV2,
323-
GovernanceAccountType::ProgramGovernanceV1,
324-
GovernanceAccountType::ProgramGovernanceV2,
325-
GovernanceAccountType::TokenGovernanceV1,
326-
GovernanceAccountType::TokenGovernanceV2,
327-
GovernanceAccountType::MintGovernanceV1,
328-
GovernanceAccountType::MintGovernanceV2,
329-
],
330-
program_id,
331-
)
360+
assert_is_valid_account_of_types(program_id, governance_info, is_governance_account_type)
332361
}
333362

334363
/// Validates args supplied to create governance account

‎governance/program/src/state/legacy.rs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,12 +137,33 @@ pub struct GovernanceV1 {
137137
pub voting_proposal_count: u16,
138138
}
139139

140-
/// Checks if the given account type is one of the Governance account types
140+
/// Checks if the given account type is one of the Governance V1 account types
141141
pub fn is_governance_v1_account_type(account_type: &GovernanceAccountType) -> bool {
142-
*account_type == GovernanceAccountType::GovernanceV1
143-
|| *account_type == GovernanceAccountType::ProgramGovernanceV1
144-
|| *account_type == GovernanceAccountType::MintGovernanceV1
145-
|| *account_type == GovernanceAccountType::TokenGovernanceV1
142+
match account_type {
143+
GovernanceAccountType::GovernanceV1
144+
| GovernanceAccountType::ProgramGovernanceV1
145+
| GovernanceAccountType::MintGovernanceV1
146+
| GovernanceAccountType::TokenGovernanceV1 => true,
147+
GovernanceAccountType::Uninitialized
148+
| GovernanceAccountType::RealmV1
149+
| GovernanceAccountType::RealmV2
150+
| GovernanceAccountType::RealmConfig
151+
| GovernanceAccountType::TokenOwnerRecordV1
152+
| GovernanceAccountType::TokenOwnerRecordV2
153+
| GovernanceAccountType::GovernanceV2
154+
| GovernanceAccountType::ProgramGovernanceV2
155+
| GovernanceAccountType::MintGovernanceV2
156+
| GovernanceAccountType::TokenGovernanceV2
157+
| GovernanceAccountType::ProposalV1
158+
| GovernanceAccountType::ProposalV2
159+
| GovernanceAccountType::SignatoryRecordV1
160+
| GovernanceAccountType::SignatoryRecordV2
161+
| GovernanceAccountType::ProposalInstructionV1
162+
| GovernanceAccountType::ProposalTransactionV2
163+
| GovernanceAccountType::VoteRecordV1
164+
| GovernanceAccountType::VoteRecordV2
165+
| GovernanceAccountType::ProgramMetadata => false,
166+
}
146167
}
147168

148169
impl IsInitialized for GovernanceV1 {

‎governance/program/src/state/realm.rs

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,34 @@ impl IsInitialized for RealmV2 {
132132
}
133133
}
134134

135+
/// Checks if the given account type is on of the Realm account types of any version
136+
pub fn is_realm_account_type(account_type: &GovernanceAccountType) -> bool {
137+
match account_type {
138+
GovernanceAccountType::RealmV1 | GovernanceAccountType::RealmV2 => true,
139+
GovernanceAccountType::GovernanceV2
140+
| GovernanceAccountType::ProgramGovernanceV2
141+
| GovernanceAccountType::MintGovernanceV2
142+
| GovernanceAccountType::TokenGovernanceV2
143+
| GovernanceAccountType::Uninitialized
144+
| GovernanceAccountType::RealmConfig
145+
| GovernanceAccountType::TokenOwnerRecordV1
146+
| GovernanceAccountType::TokenOwnerRecordV2
147+
| GovernanceAccountType::GovernanceV1
148+
| GovernanceAccountType::ProgramGovernanceV1
149+
| GovernanceAccountType::MintGovernanceV1
150+
| GovernanceAccountType::TokenGovernanceV1
151+
| GovernanceAccountType::ProposalV1
152+
| GovernanceAccountType::ProposalV2
153+
| GovernanceAccountType::SignatoryRecordV1
154+
| GovernanceAccountType::SignatoryRecordV2
155+
| GovernanceAccountType::ProposalInstructionV1
156+
| GovernanceAccountType::ProposalTransactionV2
157+
| GovernanceAccountType::VoteRecordV1
158+
| GovernanceAccountType::VoteRecordV2
159+
| GovernanceAccountType::ProgramMetadata => false,
160+
}
161+
}
162+
135163
impl RealmV2 {
136164
/// Asserts the given mint is either Community or Council mint of the Realm
137165
pub fn assert_is_valid_governing_token_mint(
@@ -254,19 +282,12 @@ impl RealmV2 {
254282
}
255283
}
256284

257-
/// Checks whether realm account exists, is initialized and owned by Governance program
285+
/// Checks whether the Realm account exists, is initialized and owned by Governance program
258286
pub fn assert_is_valid_realm(
259287
program_id: &Pubkey,
260288
realm_info: &AccountInfo,
261289
) -> Result<(), ProgramError> {
262-
assert_is_valid_account_of_types(
263-
realm_info,
264-
&[
265-
GovernanceAccountType::RealmV1,
266-
GovernanceAccountType::RealmV2,
267-
],
268-
program_id,
269-
)
290+
assert_is_valid_account_of_types(program_id, realm_info, is_realm_account_type)
270291
}
271292

272293
/// Deserializes account and checks owner program
@@ -277,7 +298,7 @@ pub fn get_realm_data(
277298
let account_type: GovernanceAccountType = try_from_slice_unchecked(&realm_info.data.borrow())?;
278299

279300
// If the account is V1 version then translate to V2
280-
if account_type == GovernanceAccountType::ProposalV1 {
301+
if account_type == GovernanceAccountType::RealmV1 {
281302
let realm_data_v1 = get_account_data::<RealmV1>(program_id, realm_info)?;
282303

283304
return Ok(RealmV2 {

‎governance/tools/src/account.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -225,18 +225,19 @@ pub fn get_account_data<T: BorshDeserialize + IsInitialized>(
225225
/// Asserts the given account is not empty, owned by the given program and of the expected type
226226
/// Note: The function assumes the account type T is stored as the first element in the account data
227227
pub fn assert_is_valid_account_of_type<T: BorshDeserialize + PartialEq>(
228-
account_info: &AccountInfo,
229-
expected_account_type: T,
230228
owner_program_id: &Pubkey,
229+
account_info: &AccountInfo,
230+
account_type: T,
231231
) -> Result<(), ProgramError> {
232-
assert_is_valid_account_of_types(account_info, &[expected_account_type], owner_program_id)
232+
assert_is_valid_account_of_types(owner_program_id, account_info, |at: &T| *at == account_type)
233233
}
234-
/// Asserts the given account is not empty, owned by the given program and one of the expected types
234+
235+
/// Asserts the given account is not empty, owned by the given program and one of the types asserted via the provided predicate function
235236
/// Note: The function assumes the account type T is stored as the first element in the account data
236-
pub fn assert_is_valid_account_of_types<T: BorshDeserialize + PartialEq>(
237-
account_info: &AccountInfo,
238-
expected_account_types: &[T],
237+
pub fn assert_is_valid_account_of_types<T: BorshDeserialize + PartialEq, F: Fn(&T) -> bool>(
239238
owner_program_id: &Pubkey,
239+
account_info: &AccountInfo,
240+
is_account_type: F,
240241
) -> Result<(), ProgramError> {
241242
if account_info.owner != owner_program_id {
242243
return Err(GovernanceToolsError::InvalidAccountOwner.into());
@@ -248,7 +249,7 @@ pub fn assert_is_valid_account_of_types<T: BorshDeserialize + PartialEq>(
248249

249250
let account_type: T = try_from_slice_unchecked(&account_info.data.borrow())?;
250251

251-
if expected_account_types.iter().all(|a| a != &account_type) {
252+
if !is_account_type(&account_type) {
252253
return Err(GovernanceToolsError::InvalidAccountType.into());
253254
};
254255

0 commit comments

Comments
 (0)