-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
revoke whitelist logic #188
Conversation
} | ||
|
||
fn get_whitelist_size_from_length(length:u8) -> usize { | ||
8 + mem::size_of::<Whitelist>() + (length as u64 * 32 as u64) as usize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(length as u64 * 32 as u64) as usize
can be simplified to length as usize * 32
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@@ -20,11 +21,10 @@ pub struct CreateWhitelistAccounts<'info> { | |||
|
|||
pub fn create_whitelist_instruction( | |||
ctx: Context<CreateWhitelistAccounts>, | |||
expected_whitelist_size: u16, | |||
length:u8, | |||
whitelist_to_add: Vec<Pubkey>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're passing it in one ix, you already have a length in this vector, no need to pass it as a parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
rfq/program/src/constants.rs
Outdated
@@ -0,0 +1 @@ | |||
pub const MAX_WHITELIST_SIZE: u8 = 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're storing constants in state files, you can check other types for examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved , added to state
@@ -3,7 +3,6 @@ use anchor_lang::prelude::*; | |||
#[account] | |||
pub struct Whitelist { | |||
pub creator: Pubkey, | |||
pub capacity: u8, | |||
pub whitelist: Vec<Pubkey>, | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add an associated rfq field, which would work as follows:
- Default when the whitelist is created
- When rfq references a whitelist, this field is set(can't associate with two rfqs)
- Automatically clean up whitelist on the rfq cleanup
- An ix to clean up a whitelist if it isn't associated with an rfq(probably wouldn't be often used but for completeness)
Please note that you can only use your own whitelist when associating it with an rfq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
whitelist.associated_rfq, | ||
rfq.key(), | ||
ProtocolError::WhitelistAssocaitionRFQMismatch | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can accidentally pass a None case if the rfq address exists and it wouldn't be cleaned up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to reuse the logic of "check whitelist account passed" used in response to rfq
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
tests/utilities/wrappers.ts
Outdated
@@ -946,12 +925,15 @@ export class Rfq { | |||
} | |||
|
|||
async cleanUp() { | |||
const rfq = await this.getData(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would slow down each cleanup with 1 additional fetch, we can pass a whitelist to the rfq constructor to avoid fetching it each time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
tests/utilities/wrappers.ts
Outdated
.rpc(); | ||
.rpc({ | ||
skipPreflight: true, | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some leftover code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
a. Remove whitelist update
b. remove remove address logic
c . set max_addresses to 20