Skip to content
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

frank/usermap #14

Merged
merged 11 commits into from
Mar 18, 2024
Merged

frank/usermap #14

merged 11 commits into from
Mar 18, 2024

Conversation

soundsonacid
Copy link
Contributor

No description provided.

Copy link
Collaborator

@jordy25519 jordy25519 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be good to add doc comments to the methods

src/usermap.rs Show resolved Hide resolved
src/usermap.rs Outdated Show resolved Hide resolved
}

pub fn get(&self, pubkey: &str) -> Option<User> {
self.usermap.get(pubkey).map(|user| user.value().clone())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible to return &User? the caller can clone if they need

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without using Cow i don't think so and i'd really rather not use Cow, cloning 4.3kb shouldn't be that expensive

pub struct Usermap {
subscribed: bool,
subscription: WebsocketProgramAccountSubscriber,
usermap: Arc<DashMap<String, User>>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is String helpful here?

Suggested change
usermap: Arc<DashMap<String, User>>,
usermap: Arc<DashMap<Pubkey, User>>,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the pubkeys come out of the events as strings, i think keeping it string makes sense

if let Some(update) = event.as_any().downcast_ref::<ProgramAccountUpdate<User>>() {
let user_data_and_slot = update.data_and_slot.clone();
let user_pubkey = update.pubkey.to_string();
if update.data_and_slot.slot > latest_slot.load(Ordering::Relaxed) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe >= ? so updates from different user in the same slot should both go through

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the data updates will still go through, it just won't go to the effort of updating the latest slot if it's the same slot

@soundsonacid soundsonacid merged commit db9c093 into drift-labs:main Mar 18, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants