Skip to content

Commit

Permalink
Better threshold UX (#4352)
Browse files Browse the repository at this point in the history
## Describe your changes

Improves UX for the DKG ceremony based on user feedback, so we can
actually tell people to use it.

This is a PR on top of #4343 to make it as easy as possible to merge.

## Issue ticket number and link

#4335

## Checklist before requesting a review

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > client-side key generation logic only
  • Loading branch information
hdevalence authored and conorsch committed May 8, 2024
1 parent c3c4360 commit 80105be
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 116 deletions.
35 changes: 25 additions & 10 deletions crates/bin/pcli/src/terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::io::{IsTerminal, Read, Write};

use anyhow::Result;
use penumbra_custody::threshold::{SigningRequest, Terminal};
use termion::input::TermRead;
use termion::{color, input::TermRead};
use tonic::async_trait;

async fn read_password(prompt: &str) -> Result<String> {
Expand Down Expand Up @@ -48,37 +48,54 @@ impl Terminal for ActualTerminal {
println!("Do you approve this {description}?");
println!("{json}");
println!("Press enter to continue");
self.next_response().await?;
self.read_line_raw().await?;
Ok(true)
}

async fn explain(&self, msg: &str) -> Result<()> {
println!("{}", msg);
fn explain(&self, msg: &str) -> Result<()> {
println!(
"{}{}{}",
color::Fg(color::Blue),
msg,
color::Fg(color::Reset)
);
Ok(())
}

async fn broadcast(&self, data: &str) -> Result<()> {
println!("{}", data);
println!(
"\n{}{}{}\n",
color::Fg(color::Yellow),
data,
color::Fg(color::Reset)
);
Ok(())
}

async fn next_response(&self) -> Result<Option<String>> {
async fn read_line_raw(&self) -> Result<String> {
// Use raw mode to allow reading more than 1KB/4KB of data at a time
// See https://unix.stackexchange.com/questions/204815/terminal-does-not-accept-pasted-or-typed-lines-of-more-than-1024-characters
use termion::raw::IntoRawMode;
tracing::debug!("about to enter raw mode for long pasted input");

print!("{}", color::Fg(color::Red));
// In raw mode, the input is not mirrored into the terminal, so we need
// to read char-by-char and echo it back.
let mut stdout = std::io::stdout().into_raw_mode()?;

let mut bytes = Vec::with_capacity(8192);
for b in std::io::stdin().bytes() {
let b = b?;
// In raw mode, we need to handle control characters ourselves
if b == 3 || b == 4 {
// Ctrl-C or Ctrl-D
return Err(anyhow::anyhow!("aborted"));
}
// In raw mode, the enter key might generate \r or \n, check either.
if b == b'\n' || b == b'\r' {
break;
}
// Store the byte we read and print it back to the terminal.
bytes.push(b);
stdout.write(&[b]).unwrap();
// Flushing may not be the most efficient but performance isn't critical here.
Expand All @@ -89,16 +106,14 @@ impl Terminal for ActualTerminal {
// We consumed a newline of some kind but didn't echo it, now print
// one out so subsequent output is guaranteed to be on a new line.
println!("");
print!("{}", color::Fg(color::Reset));

tracing::debug!("exited raw mode and returned to cooked mode");

let line = String::from_utf8(bytes)?;
tracing::debug!(?line, "read response line");

if line.is_empty() {
return Ok(None);
}
Ok(Some(line))
Ok(line)
}

async fn get_password(&self) -> Result<String> {
Expand Down
48 changes: 41 additions & 7 deletions crates/custody/src/terminal.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use anyhow::Result;
use penumbra_governance::ValidatorVoteBody;
use penumbra_proto::DomainType;
use penumbra_stake::validator::Validator;
use penumbra_transaction::TransactionPlan;
use serde::de::DeserializeOwned;
use tonic::async_trait;

#[derive(Debug, Clone)]
Expand All @@ -15,7 +17,7 @@ pub enum SigningRequest {
/// This is mainly used to accommodate the kind of interaction we have with the CLI
/// interface, but it can also be plugged in with more general backends.
#[async_trait]
pub trait Terminal {
pub trait Terminal: Sync {
/// Have a user confirm that they want to sign this transaction or other data (e.g. validator
/// definition, validator vote)
///
Expand All @@ -29,16 +31,48 @@ pub trait Terminal {
/// what subsequent data means, and what the user needs to do.
///
/// Backends can replace this with a no-op.
async fn explain(&self, msg: &str) -> Result<()>;
fn explain(&self, msg: &str) -> Result<()>;

/// Broadcast a message to other users.
async fn broadcast(&self, data: &str) -> Result<()>;

/// Wait for a response from *some* other user, it doesn't matter which.
///
/// This function should not return None spuriously, when it does,
/// it should continue to return None until a message is broadcast.
async fn next_response(&self) -> Result<Option<String>>;
/// Try to read a typed message from the terminal, retrying until
/// the message parses successfully or the user interrupts the program.
async fn next_response<D>(&self) -> Result<D>
where
D: DomainType,
anyhow::Error: From<<D as TryFrom<<D as DomainType>::Proto>>::Error>,
<D as DomainType>::Proto: DeserializeOwned,
{
loop {
// Read a line or bubble up an error if we couldn't.
let line = self.read_line_raw().await?;

let proto = match serde_json::from_str::<<D as DomainType>::Proto>(&line) {
Ok(proto) => proto,
Err(e) => {
self.explain(&format!("Error parsing response: {:#}", e))?;
self.explain("Please try again:")?;
continue;
}
};

let message = match D::try_from(proto) {
Ok(message) => message,
Err(e) => {
let e: anyhow::Error = e.into();
self.explain(&format!("Error parsing response: {:#}", e))?;
self.explain("Please try again:")?;
continue;
}
};

return Ok(message);
}
}

/// Read a single line from the terminal.
async fn read_line_raw(&self) -> Result<String>;

/// Wait for the user to supply a password.
async fn get_password(&self) -> Result<String>;
Expand Down
Loading

0 comments on commit 80105be

Please sign in to comment.