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

pcli threshold sign hangs when pasting input into the terminal #4230

Closed
hdevalence opened this issue Apr 17, 2024 · 4 comments · Fixed by #4235
Closed

pcli threshold sign hangs when pasting input into the terminal #4230

hdevalence opened this issue Apr 17, 2024 · 4 comments · Fixed by #4235
Labels
needs-refinement unclear, incomplete, or stub issue that needs work

Comments

@hdevalence
Copy link
Member

Describe the bug

pcli threshold sign hangs when pasting a message in (on MacOS, using ITerm2 and fish, if that turns out to matter)

To Reproduce
Steps to reproduce the behavior:

  1. Run pcli threshold sign
  2. Paste the coordinator's message
  3. It hangs

I'm expecting this is something about the way we're reading data from the terminal.

@github-project-automation github-project-automation bot moved this to Backlog in Penumbra Apr 17, 2024
@github-actions github-actions bot added the needs-refinement unclear, incomplete, or stub issue that needs work label Apr 17, 2024
@conorsch
Copy link
Contributor

On multiple linux machines with bash, I have never observed this behavior. Will try linux & fish too, see if we can narrow it down.

@hdevalence
Copy link
Member Author

hdevalence commented Apr 17, 2024

I tried using the standard MacOS terminal and also, incredibly frustratingly, experienced this while trying to use wc. Suspiciously, though, it looks like it pastes exactly 1024 characters (plus a newline from editing in vim):

Paste the coordinator's first message:
{"plan":{"actions":[{"output":{"value":{"amount":{"lo":"1000000"},"assetId":{"inner":"KeqcLzNx9qSH5+lcJHBB9KNW+YPrBk5dKzvPMiypahA="}},"destAddress":{"inner":"viwHqqKKW6d9RbWqNyZksghidcHjjiw+sw49Cr75I8yL/W3ELeBSz8A94EloOhb8TYBKS1geVFsMWsIEkQcaYKMBnn/ZxkgQPTVxZwpXIO0="},"rseed":"f1Ih6sdiVQZmeK+SBG2avYQKKlx4sEKolcqq9vyv0Sc=","valueBlinding":"r46ruF0TBsNAk4k54hHyzeQpc52t24wnoW3sX2IeTQQ=","proofBlindingR":"RZjTp64KUMLZKbg6JuHXtVmo2V+wzg5KNoWbRXQCoxA=","proofBlindingS":"78lPB9IK6g8UCLbpoPUOA4sVEeU7NcM3FUgCi3czOAI="}},{"spend":{"note":{"value":{"amount":{"lo":"100000000"},"assetId":{"inner":"KeqcLzNx9qSH5+lcJHBB9KNW+YPrBk5dKzvPMiypahA="}},"rseed":"th7vtpCFqLvUJCo+wodYFZ8lSadx7twlXuBw82hEefY=","address":{"inner":"xYDaNdqjbXLpgyK6ZCkfUZC1w7euFxXXuCLtj+rJZu95OCQhwNCoLAH4esSKbICQ3VOzjfE/fcn3yyYJDzz+KSze4+2xOGx8K/hcj0YEHRA="}},"position":"2121753624576","randomizer":"+wnsKrs6vtoGTp2IvBNR+ySYmNdtZrEAqgA31hC6pAE=","valueBlinding":"gyUDW5rAJ2GEhKZkOzaC5XQg+6ZefjkI1JKZzg1v6AE=","proofBlindingR":"OevjrpofQuhS0k2hp7rU5X9c5UTN6^C
hdevalence@phobos-3 ~ % cat | wc
{"plan":{"actions":[{"output":{"value":{"amount":{"lo":"1000000"},"assetId":{"inner":"KeqcLzNx9qSH5+lcJHBB9KNW+YPrBk5dKzvPMiypahA="}},"destAddress":{"inner":"viwHqqKKW6d9RbWqNyZksghidcHjjiw+sw49Cr75I8yL/W3ELeBSz8A94EloOhb8TYBKS1geVFsMWsIEkQcaYKMBnn/ZxkgQPTVxZwpXIO0="},"rseed":"f1Ih6sdiVQZmeK+SBG2avYQKKlx4sEKolcqq9vyv0Sc=","valueBlinding":"r46ruF0TBsNAk4k54hHyzeQpc52t24wnoW3sX2IeTQQ=","proofBlindingR":"RZjTp64KUMLZKbg6JuHXtVmo2V+wzg5KNoWbRXQCoxA=","proofBlindingS":"78lPB9IK6g8UCLbpoPUOA4sVEeU7NcM3FUgCi3czOAI="}},{"spend":{"note":{"value":{"amount":{"lo":"100000000"},"assetId":{"inner":"KeqcLzNx9qSH5+lcJHBB9KNW+YPrBk5dKzvPMiypahA="}},"rseed":"th7vtpCFqLvUJCo+wodYFZ8lSadx7twlXuBw82hEefY=","address":{"inner":"xYDaNdqjbXLpgyK6ZCkfUZC1w7euFxXXuCLtj+rJZu95OCQhwNCoLAH4esSKbICQ3VOzjfE/fcn3yyYJDzz+KSze4+2xOGx8K/hcj0YEHRA="}},"position":"2121753624576","randomizer":"+wnsKrs6vtoGTp2IvBNR+ySYmNdtZrEAqgA31hC6pAE=","valueBlinding":"gyUDW5rAJ2GEhKZkOzaC5XQg+6ZefjkI1JKZzg1v6AE=","proofBlindingR":"OevjrpofQuhS0k2hp7rU5X9c5UTN6^C
hdevalence@phobos-3 ~ % 
hdevalence@phobos-3 ~ % 
hdevalence@phobos-3 ~ % vim /tmp/test
hdevalence@phobos-3 ~ % wc /tmp/test  
       1       1    1025 /tmp/test

@hdevalence
Copy link
Member Author

@hdevalence
Copy link
Member Author

chatgpt

Certainly! Below is an example using Rust and the Termion crate, where the program enters cbreak mode, reads a single line from standard input, and then returns the terminal to its original state:

extern crate termion;

use termion::input::TermRead;
use termion::raw::IntoRawMode;
use std::io::{self, Write};

fn main() {
    // Set up the terminal in cbreak mode
    let stdin = io::stdin();
    let mut stdout = io::stdout().into_raw_mode().unwrap();

    writeln!(stdout, "Please enter a line: ").unwrap();
    stdout.flush().unwrap();

    // Read a single line
    if let Some(Ok(line)) = stdin.lock().lines().next() {
        writeln!(stdout, "You entered: {}", line).unwrap();
    } else {
        writeln!(stdout, "Error reading line").unwrap();
    }

    // Terminal will be restored to its original state when `stdout` is dropped.
}

In this code, into_raw_mode() puts the terminal into cbreak mode. The TermRead trait extension provides a .lines() iterator over the standard input, allowing us to read a line. When the stdout variable goes out of scope, the terminal is automatically restored to its original state due to the Drop implementation of the raw mode guard returned by into_raw_mode().

hdevalence added a commit that referenced this issue Apr 18, 2024
hdevalence added a commit that referenced this issue Apr 18, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in Penumbra Apr 18, 2024
hdevalence added a commit that referenced this issue Apr 29, 2024
## Describe your changes

Cherry picks 1376d4b onto the 0.73
release branch so that we can deploy a patch release of pcli ahead of a
network upgrade.

## Issue ticket number and link

#4230 

## 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:

  > only changes the pcli terminal handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-refinement unclear, incomplete, or stub issue that needs work
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants