Skip to content

feat: add chat-cli /add with fast group-commit timing - #208

Open
mchenani wants to merge 1 commit into
mch/libchat-refactoring-connectfrom
mch/libchat-refactoring-add-member-to-group
Open

feat: add chat-cli /add with fast group-commit timing#208
mchenani wants to merge 1 commit into
mch/libchat-refactoring-connectfrom
mch/libchat-refactoring-add-member-to-group

Conversation

@mchenani

Copy link
Copy Markdown
Contributor

No description provided.

@jazzz jazzz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Adjusting the DeMLS timers can be tempting however can due more harm than good.

My preference is that developers do not have access to these values in the future given the complexity involved.

the proposed timers would support ~2-3 members IIRC.

As a living example for how developers use the library, I would lean towards simplicity and clarity and focus on fixing the problem at the root (in DeMLS)

Comment thread bin/chat-cli/src/app.rs
Comment on lines +362 to +364
let chat_id = self.state.active_chat.clone().ok_or_else(|| {
anyhow::anyhow!("No active conversation. Use /new to create a group.")
})?;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Sand] Ownership does not appear to be require for chat_id. Use as_ref instead to avoid the copy

Comment thread bin/chat-cli/src/main.rs
Comment on lines +37 to +41
/// Fast GroupV2 timing so `/add` commits in ~1s instead of ~60s — for local
/// demos and tests. These are the vetted values from the library's group
/// tests; they are deliberately aggressive and not appropriate for a
/// high-latency network (hence `--group-commit auto` keeps defaults there).
fn fast_group_v2_config() -> GroupV2Config {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[!] Lowering timers, limits the number of members. If DeMLS proposals cannot be processed in the time allowed, then a deadlock occurs.

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