ledger-tool: Move blockstore arg parsing to open_blockstore()

[steviez]

Problem

The open_blockstore() helper currently takes multiple configurable options. While the arguments are parsed at a high enough scope in main.rs to avoid repeated calls, this parsing is duplicated in program.rs and bigtable.rs. The repeated parsing is redundant, and also prone to having to missing an arg (as was the case with bigtable not having wal_recovery_mode).

Summary of Changes

Move the argument parsing to within open_blockstore() to avoid repeated code and to reduce the number of parameters getting passed around.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d500eb6) 81.8% compared to head (1add55c) 81.8%.
Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master   #34596     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         824      824             
  Lines      222240   222235      -5     
=========================================
- Hits       181882   181852     -30     
- Misses      40358    40383     +25     

[CriesofCarrots]

Just pointing out that in various places, cli flags will start to have an effect which did not previously. I'm approving on the assumption that there's no substantive reason they were previously not being respected, but please confirm :)

[steviez]

Just pointing out that in various places, cli flags will start to have an effect which did not previously.

Yep, good call out; this was known but I probably could have been more explicit in stating that in the description.

I'm approving on the assumption that there's no substantive reason they were previously not being respected, but please confirm :)

There is no reason that I'm aware of for why we'd want to ignore the flags; I believe just an oversight when new flags or new commands were added. We have good defaults, but if someone really wants to set a certain value for any of these flags, there is no reason not to respect it. The only option that is really special on a per-command basis is the access mode, and that value is still explicitly specified on a per-command basis such that we run with the most restrictive access mode possible.

In general, another place where there is inconsistencies in which flags are available are the various process options. Granted some of those might be special to create-snapshot, but this is another instance where we have repeated code that has likely caused some inconsistencies. I also plan on consolidating that argument parsing into a single place for the sake of consistency between commands.