Update main.rs to improve Properties tab #154
Conversation
| if reindex { | ||
| match fs::remove_file("/root/.bitcoin/requires.reindex") { | ||
| Ok(()) => (), | ||
| Err(e) if e.kind() == std::io::ErrorKind::NotFound => (), | ||
| a => a?, | ||
| } | ||
| } |
There was a problem hiding this comment.
Why create a new conditional block here to delete the requires.reindex file when there is already a conditional block above used to add the "-reindex" arg?
| @@ -431,7 +560,6 @@ fn inner_main(reindex: bool, reindex_chainstate: bool) -> Result<(), Box<dyn Err | |||
| format!("-onion={}:9050", var("EMBASSY_IP")?), | |||
| format!("-externalip={}", peer_addr), | |||
| "-datadir=/root/.bitcoin".to_owned(), | |||
| "-deprecatedrpc=warnings".to_owned(), | |||
There was a problem hiding this comment.
Why is the "deprecatedrpc=warnings" arg being removed here?
While Bitcoin 28 has not yet shipped for bitcoind-startos, this arg is currently required for some dependencies like LND until LND 18.4 is released (currently on rc1).
| let mempool_info = std::process::Command::new("bitcoin-cli") | ||
| .arg("-conf=/root/.bitcoin/bitcoin.conf") | ||
| .arg("getmempoolinfo") | ||
| .output()?; | ||
|
|
||
| if mempool_info.status.success() { | ||
| let mempool_data: serde_json::Value = serde_json::from_slice(&mempool_info.stdout)?; | ||
|
|
||
| let max_mempool = mempool_data["maxmempool"].as_u64().unwrap_or(0) as f64 / 1024_f64.powf(2.0); // Convert to MB | ||
| let mempool_usage = mempool_data["usage"].as_u64().unwrap_or(0) as f64 / 1024_f64.powf(2.0); // Convert to MB | ||
| let mempool_percent = if max_mempool > 0.0 { | ||
| (mempool_usage / max_mempool) * 100.0 | ||
| } else { | ||
| 0.0 | ||
| }; | ||
| let tx_count = mempool_data["size"].as_u64().unwrap_or(0); // Number of transactions | ||
|
|
||
| // Insert the simplified mempool summary | ||
| stats.insert( | ||
| Cow::from("Mempool Summary"), | ||
| Stat { | ||
| value_type: "string", | ||
| value: format!( | ||
| "{:.2}/{:.2} MB ({:.2}%), {} Transactions", | ||
| mempool_usage, max_mempool, mempool_percent, tx_count | ||
| ), | ||
| description: Some(Cow::from("Summary of current mempool usage and transaction count")), | ||
| copyable: false, | ||
| qr: false, | ||
| masked: false, | ||
| }, | ||
| ); | ||
| } else { | ||
| eprintln!( | ||
| "Error retrieving mempool info: {}", | ||
| std::str::from_utf8(&mempool_info.stderr).unwrap_or("UNKNOWN ERROR") | ||
| ); | ||
| } |
There was a problem hiding this comment.
I'm not sure if it makes sense to include this information in Properties when users can get this and more detailed information about their mempool in a better format by using mempool-startos.
|
|
||
| fn inner_main(reindex: bool, reindex_chainstate: bool) -> Result<(), Box<dyn Error>> { |
There was a problem hiding this comment.
This parameter is needed for the Reindex Chainstate action. Is there a reason this parameter and all its subsequent uses are being removed?
| Stat { | ||
| value_type: "string", | ||
| value: uptime_formatted, | ||
| description: Some(Cow::from("Total time the Bitcoin node has been running")), |
There was a problem hiding this comment.
This description is a bit misleading as it might leave a user thinking their node has only been running for a short time in total.
Maybe a description like "Total time Bitcoin has been running since it was last started"?
| if blockchain_info.status.success() { | ||
| let blockchain_data: serde_json::Value = serde_json::from_slice(&blockchain_info.stdout)?; | ||
| let current_height = blockchain_data["blocks"].as_u64().unwrap_or(0); | ||
|
|
||
| // Calculate total supply based on height and halving intervals | ||
| let halving_interval = 210_000; | ||
| let mut subsidy = 50.0; // Initial subsidy in BTC | ||
| let mut total_supply = 0.0; | ||
| let mut remaining_height = current_height; | ||
|
|
||
| while remaining_height > 0 { | ||
| let blocks_in_this_epoch = remaining_height.min(halving_interval); | ||
| total_supply += blocks_in_this_epoch as f64 * subsidy; | ||
| remaining_height = remaining_height.saturating_sub(halving_interval); | ||
| subsidy /= 2.0; | ||
| } | ||
|
|
There was a problem hiding this comment.
Given the imprecision of this calculation of issuance compared to gettxoutsetinfo (even if it is small as a percentage) I don't think it makes sense to include this metric in Properties.
| value: format!( | ||
| "{}/{} ({:.2}%)", | ||
| current_blocks, total_headers, sync_progress | ||
| ), |
There was a problem hiding this comment.
This is a great improvement to make the Properties more concise. Nicely done. However I would add some spacing here for better readability.
| value: format!( | |
| "{}/{} ({:.2}%)", | |
| current_blocks, total_headers, sync_progress | |
| ), | |
| value: format!( | |
| "{} / {} ({:.2}%)", | |
| current_blocks, total_headers, sync_progress | |
| ), |
Properties added:
I also consolidated the blockchain sync summary to one line to save screen real estate.
Here is an image of what it looks like:
Reasons 'total bitcoin supply' may not be exactly the same as gettxoutinfo:
However overall this is a great at a glance metric imo to keep a check on things (with a fraction of a percent difference), especially since gettxoutsetinfo is resource and time intensive.