Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

ledger-tool does *not* fastboot by default #34228

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Nov 27, 2023

Problem

Fastboot is great for the validator, but can expose some sharp edges for ledger-tool. If a validator is already running, then calling ledger-tool will fail (on the subcommand that use accounts) due to the default fastboot value (since it tries to get primary access to the ledger). The error message does not make it obvious what the problem is, nor how to fix it.

Summary of Changes

Do not fastboot by default for ledger-tool.

This is safe, and retains the expectation that ledger-tool should not modify anything by default. Users can still opt into fastboot with ledger-tool.

@brooksprumo brooksprumo self-assigned this Nov 27, 2023
@brooksprumo brooksprumo marked this pull request as ready for review November 27, 2023 17:52
Copy link
Contributor

@steviez steviez left a comment

Choose a reason for hiding this comment

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

Thanks for the fix @brooksprumo & thanks @seanyoung for uncovering / bisecting to pinpoint the problem!

Copy link
Contributor

@seanyoung seanyoung left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for fixing quickly

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Merging #34228 (7cfc863) into master (c6451e9) will increase coverage by 0.0%.
Report is 4 commits behind head on master.
The diff coverage is 0.0%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #34228   +/-   ##
=======================================
  Coverage    81.9%    81.9%           
=======================================
  Files         819      819           
  Lines      219338   219341    +3     
=======================================
+ Hits       179695   179743   +48     
+ Misses      39643    39598   -45     

@brooksprumo brooksprumo merged commit 5c7ab5d into solana-labs:master Nov 27, 2023
17 checks passed
@brooksprumo brooksprumo deleted the fastboot/ledger-tool branch November 27, 2023 18:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants