-
Notifications
You must be signed in to change notification settings - Fork 23
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
feat: finetune leader board #2021
Conversation
@@ -127,7 +127,7 @@ pub async fn process_new_order( | |||
network: Network, | |||
oracle_pk: XOnlyPublicKey, | |||
) -> Result<Order> { | |||
tracing::info!( | |||
tracing::trace!( |
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.
👍 Ah, right. Because of our constant limit orders.
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.
LGTM, although I would opt to model the contact details and nickname as Option<String>
.
Is the nickname mandatory? I would maybe make it mandatory and not customisable, to avoid having to moderate any names. The user is just able to roll their nickname as many times as they want.
Also, if we find duplicates in the leader board, we could just append the start of their pubkey, just like Discord does: fried-otter#a1b7
.
@@ -0,0 +1,2 @@ | |||
ALTER TABLE | |||
users ADD COLUMN nickname TEXT NOT NULL DEFAULT ''; |
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.
🔧 Missing newline.
coordinator/src/db/user.rs
Outdated
// If no name or contact has been provided we default to empty string | ||
let contact = contact.unwrap_or_default(); | ||
let nickname = nickname.unwrap_or_default(); |
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.
🤔 What is the advantage of this?
I think it would be better to model this as a nullable type, so that it's more explicit when the user has decided to not provide one of these.
} | ||
|
||
#[tokio::main(flavor = "current_thread")] | ||
pub async fn update_nickname(nickname: String) -> Result<()> { |
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.
❓ On the topic of the nullable nickname, I think FRB should be able to accept an Option<String>
here, right?
@@ -31,6 +31,7 @@ ln-dlc-storage = { path = "../../crates/ln-dlc-storage" } | |||
openssl = { version = "0.10.60", features = ["vendored"] } | |||
orderbook-client = { path = "../../crates/orderbook-client" } | |||
parking_lot = { version = "0.12.1" } | |||
petname = "1.1.3" |
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.
👍 Haha, great.
@@ -0,0 +1,26 @@ | |||
pub fn get_new_name() -> String { | |||
let names = petname::Petnames::default(); | |||
let input = names.generate_one(2, " "); |
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 would expect it to be separated with a hyphen!
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.
fn uppercase_first_characters(input: &str, separator: char) -> String { | ||
let mut parts = input.splitn(2, separator); | ||
if let (Some(first), Some(second)) = (parts.next(), parts.next()) { | ||
return format!( | ||
"{} {}", | ||
uppercase_first_character(first), | ||
uppercase_first_character(second) | ||
); | ||
} | ||
|
||
input.to_string() | ||
} |
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 prefer lowercase!
This produces quite some log spam on the coordinator for not much value Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
Signed-off-by: Philipp Hoenisch <philipp@coblox.tech>
4f75584
to
9e8c349
Compare
Leader board has now a nickname which can be configured in the app (or is random generated on registration).
There is no way to opt-out of the leader board at the moment.
Also, the leaderboard api allows for defining a start and end date.
e.g. use with
curl 'localhost:8000/api/leaderboard?top=3&category=Pnl&start=2024-02-11&end=2024-02-13' -v | jq .
resolves #341