Skip to content

Commit d3126b0

Browse files
tchardingtcharding
committed
Merge #352: Fix verify
2a6eec7 Make verfiy fail on error (Jamil Lambert, PhD) 31382bf Verify search for whole word (Jamil Lambert, PhD) e2c78e4 Fix verify RPC method tags (Jamil Lambert, PhD) Pull request description: #351 described the problem - During the updating of the RPCs some of them changed from having a model to not or vice versa. There is one typo. There is also a duplicate entry for `enumeratesigners` which was added to `signer` and not removed from `wallet`. - Fix the tags in `verify` to match the correct RPC method definitions. - There were false positives in verify when the RPC name was a substring of another identifier. - Change the regex to match whole words. - `verify` does not fail when there are detected problems, it prints `Correct ✓` and continues. - Change the tool so that it prints `Incorrect ✗` if there are inconsistencies. - Run all tests to the end and then if there were any errors, return a non-zero exit code. - Change `verify_correct_methods` to return a `Result<()>` instead of `Result<(bool)>` to match the other functions. Closes #351 ACKs for top commit: tcharding: ACK 2a6eec7 Tree-SHA512: a3c43af866f02fd7ec18ab608935fdd4e05cd9cec4e91cddacf6c4574791531aae0394db194c5c31e869402dd2a049164b22b24dcb6e62e582255a3184182d1d
2 parents 0e3f028 + 2a6eec7 commit d3126b0

File tree

11 files changed

+89
-64
lines changed

11 files changed

+89
-64
lines changed

‎verify/src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ pub fn grep_for_re_export(path: &Path, s: &str) -> Result<bool> {
183183
.with_context(|| format!("Failed to grep for string in {}", path.display()))?;
184184
let reader = io::BufReader::new(file);
185185

186-
let s = format!("{}[,}};]", &s);
187-
let re = Regex::new(&s)?;
186+
let pattern = format!(r"\b{}[,}};]", regex::escape(s));
187+
let re = Regex::new(&pattern)?;
188188

189189
for line in reader.lines() {
190190
let line = line?;

‎verify/src/main.rs‎

Lines changed: 50 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,37 +53,48 @@ fn main() -> Result<()> {
5353
}
5454

5555
fn verify_all_versions(test_output: Option<&String>, quiet: bool) -> Result<()> {
56+
let mut any_failed = false;
5657
for version in VERSIONS {
5758
println!("\nVerifying for Bitcoin Core version {} ...", version);
58-
verify_version(version, test_output, quiet)?;
59+
if verify_version(version, test_output, quiet).is_err() {
60+
any_failed = true;
61+
}
62+
}
63+
if any_failed {
64+
return Err(anyhow::anyhow!("verification failed for one or more versions"))
5965
}
6066
Ok(())
6167
}
6268

6369
fn verify_version(version: Version, test_output: Option<&String>, quiet: bool) -> Result<()> {
70+
let mut failures = 0;
71+
6472
let s = format!("{}::METHOD data", version);
6573
let msg = format!("Checking that the {} list is correct", s);
6674
check(&msg, quiet);
67-
let correct = verify_correct_methods(version, method::all_methods(version), &s)?;
68-
close(correct, quiet);
69-
if !correct {
70-
process::exit(1);
75+
match verify_correct_methods(version, method::all_methods(version), &s) {
76+
Ok(()) => close(true, quiet),
77+
Err(e) => { if !quiet { eprintln!("{}", e); } close(false, quiet); failures += 1; }
7178
}
7279

7380
let s = "rustdoc version specific rustdocs";
7481
let msg = format!("Checking that the {} list is correct", s);
7582
check(&msg, quiet);
76-
let correct = verify_correct_methods(version, versioned::all_methods(version)?, s)?;
77-
close(correct, quiet);
78-
if !correct {
79-
process::exit(1);
83+
match verify_correct_methods(version, versioned::all_methods(version)?, s) {
84+
Ok(()) => close(true, quiet),
85+
Err(e) => { if !quiet { eprintln!("{}", e); } close(false, quiet); failures += 1; }
8086
}
8187

8288
let msg = "Checking that the status claimed in the version specific rustdocs is correct";
8389
check(msg, quiet);
84-
verify_status(version, test_output)?;
85-
close(correct, quiet);
90+
match verify_status(version, test_output) {
91+
Ok(()) => close(true, quiet),
92+
Err(e) => { if !quiet { eprintln!("{}", e); } close(false, quiet); failures += 1; }
93+
}
8694

95+
if failures > 0 {
96+
return Err(anyhow::anyhow!("verification failed ({} check(s) failed)", failures));
97+
}
8798
Ok(())
8899
}
89100

@@ -94,40 +105,53 @@ fn check(msg: &str, quiet: bool) {
94105
}
95106

96107
fn close(correct: bool, quiet: bool) {
97-
if correct && !quiet {
108+
if quiet { return; }
109+
if correct {
98110
println!("Correct \u{2713} \n");
111+
} else {
112+
println!("\u{001b}[31mIncorrect \u{2717}\u{001b}[0m \n");
99113
}
100114
}
101115

102116
/// Verifies that the correct set of methods are documented.
103-
fn verify_correct_methods(version: Version, methods: Vec<String>, msg: &str) -> Result<bool> {
117+
fn verify_correct_methods(version: Version, methods: Vec<String>, msg: &str) -> Result<()> {
104118
let ssot = ssot::all_methods(version)?;
105119
let want = ssot.iter().map(|s| s.as_str()).collect::<Vec<&str>>();
106120
let got = methods.iter().map(|s| s.as_str()).collect::<Vec<&str>>();
107-
Ok(verify::correct_methods(&got, &want, msg))
121+
if !verify::correct_methods(&got, &want, msg) {
122+
return Err(anyhow::anyhow!("incorrect {}", msg));
123+
}
124+
Ok(())
108125
}
109126

110127
/// Verifies that the status we claim is correct.
111128
fn verify_status(version: Version, test_output: Option<&String>) -> Result<()> {
112129
let methods = versioned::methods_and_status(version)?;
130+
let mut failures = 0;
113131
for method in methods {
114132
match method.status {
115133
Status::Done => {
116-
check_types_exist_if_required(version, &method.name)?;
134+
if check_types_exist_if_required(version, &method.name).is_err() {
135+
failures += 1;
136+
}
117137

118138
if let Some(test_output) = test_output {
119-
if !check_integration_test_crate::test_exists(version, &method.name, test_output)? {
139+
if check_integration_test_crate::test_exists(version, &method.name, test_output).is_err() {
120140
eprintln!("missing integration test: {}", method.name);
141+
failures += 1;
121142
}
122143
}
123144
}
124145
Status::Untested => {
125-
check_types_exist_if_required(version, &method.name)?;
146+
if check_types_exist_if_required(version, &method.name).is_err() {
147+
failures += 1;
148+
}
126149

127150
// Make sure we didn't forget to mark as tested after implementing integration test.
128151
if let Some(test_output) = test_output {
129-
if check_integration_test_crate::test_exists(version, &method.name, test_output)? {
152+
if check_integration_test_crate::test_exists(version, &method.name, test_output).is_ok() {
130153
eprintln!("found integration test for untested method: {}", method.name);
154+
failures += 1;
131155
}
132156
}
133157
}
@@ -137,15 +161,20 @@ fn verify_status(version: Version, test_output: Option<&String>) -> Result<()> {
137161

138162
if versioned::type_exists(version, &method.name)? && !versioned::requires_type(version, &method.name)? {
139163
eprintln!("return type found but method is omitted or TODO: {}", output_method(out));
164+
failures += 1;
140165
}
141166

142167
if model::type_exists(version, &method.name)? && !model::requires_type(version, &method.name)? {
143168
eprintln!("model type found but method is omitted or TODO: {}", output_method(out));
169+
failures += 1;
144170
}
145171
}
146172
}
147173
}
148174

175+
if failures > 0 {
176+
return Err(anyhow::anyhow!("status verification failed ({} issue(s))", failures));
177+
}
149178
Ok(())
150179
}
151180

@@ -154,12 +183,15 @@ fn check_types_exist_if_required(version: Version, method_name: &str) -> Result<
154183

155184
if versioned::requires_type(version, method_name)? && !versioned::type_exists(version, method_name)? {
156185
eprintln!("missing return type: {}", output_method(out));
186+
return Err(anyhow::anyhow!("missing return type"));
157187
}
158188
if model::requires_type(version, method_name)? && !model::type_exists(version, method_name)? {
159189
eprintln!("missing model type: {}", output_method(out));
190+
return Err(anyhow::anyhow!("missing model type"));
160191
}
161192
if model::type_exists(version, method_name)? && !model::requires_type(version, method_name)? {
162193
eprintln!("found model type when none expected: {}", output_method(out));
194+
return Err(anyhow::anyhow!("unexpected model type"));
163195
}
164196
Ok(())
165197
}

‎verify/src/method/v21.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ pub const METHODS: &[Method] = &[
161161
"sign_raw_transaction_with_wallet",
162162
),
163163
Method::new_nothing("unloadwallet", "unload_wallet"),
164-
Method::new_no_model("upgradewallet", "UpgradeWalled", "upgrade_wallet"),
164+
Method::new_no_model("upgradewallet", "UpgradeWallet", "upgrade_wallet"),
165165
Method::new_modelled(
166166
"walletcreatefundedpsbt",
167167
"WalletCreateFundedPsbt",

‎verify/src/method/v22.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ pub const METHODS: &[Method] = &[
164164
"sign_raw_transaction_with_wallet",
165165
),
166166
Method::new_nothing("unloadwallet", "unload_wallet"),
167-
Method::new_no_model("upgradewallet", "UpgradeWalled", "upgrade_wallet"),
167+
Method::new_no_model("upgradewallet", "UpgradeWallet", "upgrade_wallet"),
168168
Method::new_modelled(
169169
"walletcreatefundedpsbt",
170170
"WalletCreateFundedPsbt",

‎verify/src/method/v23.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ pub const METHODS: &[Method] = &[
1212
Method::new_modelled("getblockchaininfo", "GetBlockchainInfo", "get_blockchain_info"),
1313
Method::new_modelled("getblockcount", "GetBlockCount", "get_block_count"),
1414
Method::new_modelled("getblockfilter", "GetBlockFilter", "get_block_filter"),
15-
Method::new_modelled("getblockfrompeer", "GetBlockFromPeer", "get_block_from_peer"),
15+
Method::new_nothing("getblockfrompeer", "get_block_from_peer"),
1616
Method::new_modelled("getblockhash", "GetBlockHash", "get_block_hash"),
1717
Method::new_modelled("getblockheader", "GetBlockHeader", "get_block_header"),
1818
Method::new_modelled("getblockstats", "GetBlockStats", "get_block_stats"),
@@ -84,7 +84,6 @@ pub const METHODS: &[Method] = &[
8484
Method::new_nothing("signrawtransactionwithkey", "sign_raw_transaction_with_key"),
8585
Method::new_nothing("testmempoolaccept", "test_mempool_accept"),
8686
Method::new_modelled("utxoupdatepsbt", "UtxoUpdatePsbt", "utxo_update_psbt"),
87-
Method::new_modelled("enumeratesigners", "EnumerateSigners", "enumerate_signers"),
8887
Method::new_modelled("createmultisig", "CreateMultisig", "create_multisig"),
8988
Method::new_modelled("deriveaddresses", "DeriveAddresses", "derive_addresses"),
9089
Method::new_nothing("estimatesmartfee", "estimate_smart_fee"),
@@ -132,7 +131,7 @@ pub const METHODS: &[Method] = &[
132131
Method::new_no_model("listdescriptors", "ListDescriptors", "list_descriptors"),
133132
Method::new_no_model("listlabels", "ListLabels", "list_labels"),
134133
Method::new_modelled("listlockunspent", "ListLockUnspent", "list_lock_unspent"),
135-
Method::new_modelled("newkeypool", "NewKeyPool", "new_key_pool"),
134+
Method::new_nothing("newkeypool", "new_key_pool"),
136135
Method::new_modelled("psbtbumpfee", "PsbtBumpFee", "psbt_bump_fee"),
137136
Method::new_modelled(
138137
"listreceivedbyaddress",
@@ -149,7 +148,7 @@ pub const METHODS: &[Method] = &[
149148
Method::new_no_model("lockunspent", "LockUnspent", "lock_unspent"),
150149
Method::new_nothing("removeprunedfunds", "remove_pruned_funds"),
151150
Method::new_modelled("rescanblockchain", "RescanBlockchain", "rescan_blockchain"),
152-
Method::new_modelled("restorewallet", "RestoreWallet", "restore_wallet"),
151+
Method::new_no_model("restorewallet", "RestoreWallet", "restore_wallet"),
153152
Method::new_modelled("send", "Send", "send"),
154153
Method::new_modelled("sendmany", "SendMany", "send_many"),
155154
Method::new_modelled("sendtoaddress", "SendToAddress", "send_to_address"),
@@ -164,7 +163,7 @@ pub const METHODS: &[Method] = &[
164163
"sign_raw_transaction_with_wallet",
165164
),
166165
Method::new_nothing("unloadwallet", "unload_wallet"),
167-
Method::new_no_model("upgradewallet", "UpgradeWalled", "upgrade_wallet"),
166+
Method::new_no_model("upgradewallet", "UpgradeWallet", "upgrade_wallet"),
168167
Method::new_modelled(
169168
"walletcreatefundedpsbt",
170169
"WalletCreateFundedPsbt",

‎verify/src/method/v24.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ pub const METHODS: &[Method] = &[
1212
Method::new_modelled("getblockchaininfo", "GetBlockchainInfo", "get_blockchain_info"),
1313
Method::new_modelled("getblockcount", "GetBlockCount", "get_block_count"),
1414
Method::new_modelled("getblockfilter", "GetBlockFilter", "get_block_filter"),
15-
Method::new_modelled("getblockfrompeer", "GetBlockFromPeer", "get_block_from_peer"),
15+
Method::new_nothing("getblockfrompeer", "get_block_from_peer"),
1616
Method::new_modelled("getblockhash", "GetBlockHash", "get_block_hash"),
1717
Method::new_modelled("getblockheader", "GetBlockHeader", "get_block_header"),
1818
Method::new_modelled("getblockstats", "GetBlockStats", "get_block_stats"),
@@ -85,7 +85,6 @@ pub const METHODS: &[Method] = &[
8585
Method::new_nothing("signrawtransactionwithkey", "sign_raw_transaction_with_key"),
8686
Method::new_nothing("testmempoolaccept", "test_mempool_accept"),
8787
Method::new_modelled("utxoupdatepsbt", "UtxoUpdatePsbt", "utxo_update_psbt"),
88-
Method::new_modelled("enumeratesigners", "EnumerateSigners", "enumerate_signers"),
8988
Method::new_modelled("createmultisig", "CreateMultisig", "create_multisig"),
9089
Method::new_modelled("deriveaddresses", "DeriveAddresses", "derive_addresses"),
9190
Method::new_nothing("estimatesmartfee", "estimate_smart_fee"),
@@ -134,7 +133,7 @@ pub const METHODS: &[Method] = &[
134133
Method::new_no_model("listlabels", "ListLabels", "list_labels"),
135134
Method::new_modelled("listlockunspent", "ListLockUnspent", "list_lock_unspent"),
136135
Method::new_no_model("migratewallet", "MigrateWallet", "migrate_wallet"),
137-
Method::new_modelled("newkeypool", "NewKeyPool", "new_key_pool"),
136+
Method::new_nothing("newkeypool", "new_key_pool"),
138137
Method::new_modelled("psbtbumpfee", "PsbtBumpFee", "psbt_bump_fee"),
139138
Method::new_modelled(
140139
"listreceivedbyaddress",
@@ -151,7 +150,7 @@ pub const METHODS: &[Method] = &[
151150
Method::new_no_model("lockunspent", "LockUnspent", "lock_unspent"),
152151
Method::new_nothing("removeprunedfunds", "remove_pruned_funds"),
153152
Method::new_modelled("rescanblockchain", "RescanBlockchain", "rescan_blockchain"),
154-
Method::new_modelled("restorewallet", "RestoreWallet", "restore_wallet"),
153+
Method::new_no_model("restorewallet", "RestoreWallet", "restore_wallet"),
155154
Method::new_modelled("send", "Send", "send"),
156155
Method::new_modelled("sendall", "SendAll", "send_all"),
157156
Method::new_modelled("sendmany", "SendMany", "send_many"),
@@ -168,7 +167,7 @@ pub const METHODS: &[Method] = &[
168167
),
169168
Method::new_modelled("simulaterawtransaction", "SimulateRawTransaction", "simulate_raw_transaction"),
170169
Method::new_nothing("unloadwallet", "unload_wallet"),
171-
Method::new_no_model("upgradewallet", "UpgradeWalled", "upgrade_wallet"),
170+
Method::new_no_model("upgradewallet", "UpgradeWallet", "upgrade_wallet"),
172171
Method::new_modelled(
173172
"walletcreatefundedpsbt",
174173
"WalletCreateFundedPsbt",

‎verify/src/method/v25.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ pub const METHODS: &[Method] = &[
1212
Method::new_modelled("getblockchaininfo", "GetBlockchainInfo", "get_blockchain_info"),
1313
Method::new_modelled("getblockcount", "GetBlockCount", "get_block_count"),
1414
Method::new_modelled("getblockfilter", "GetBlockFilter", "get_block_filter"),
15-
Method::new_modelled("getblockfrompeer", "GetBlockFromPeer", "get_block_from_peer"),
15+
Method::new_nothing("getblockfrompeer", "get_block_from_peer"),
1616
Method::new_modelled("getblockhash", "GetBlockHash", "get_block_hash"),
1717
Method::new_modelled("getblockheader", "GetBlockHeader", "get_block_header"),
1818
Method::new_modelled("getblockstats", "GetBlockStats", "get_block_stats"),
@@ -36,7 +36,7 @@ pub const METHODS: &[Method] = &[
3636
Method::new_nothing("preciousblock", "precious_block"),
3737
Method::new_no_model("pruneblockchain", "PruneBlockchain", "prune_blockchain"),
3838
Method::new_no_model("savemempool", "SaveMempool", "save_mempool"),
39-
Method::new_modelled("scanblocks", "ScanBlocks", "scan_blocks"),
39+
Method::new_modelled("scanblocks", "ScanBlocksStart", "scan_blocks"),
4040
Method::new_modelled("scantxoutset", "ScanTxOutSet", "scan_tx_out_set"),
4141
Method::new_no_model("verifychain", "VerifyChain", "verify_chain"),
4242
Method::new_modelled("verifytxoutproof", "VerifyTxOutProof", "verify_tx_out_proof"),
@@ -86,7 +86,6 @@ pub const METHODS: &[Method] = &[
8686
Method::new_nothing("signrawtransactionwithkey", "sign_raw_transaction_with_key"),
8787
Method::new_nothing("testmempoolaccept", "test_mempool_accept"),
8888
Method::new_modelled("utxoupdatepsbt", "UtxoUpdatePsbt", "utxo_update_psbt"),
89-
Method::new_modelled("enumeratesigners", "EnumerateSigners", "enumerate_signers"),
9089
Method::new_modelled("createmultisig", "CreateMultisig", "create_multisig"),
9190
Method::new_modelled("deriveaddresses", "DeriveAddresses", "derive_addresses"),
9291
Method::new_nothing("estimatesmartfee", "estimate_smart_fee"),
@@ -135,7 +134,7 @@ pub const METHODS: &[Method] = &[
135134
Method::new_no_model("listlabels", "ListLabels", "list_labels"),
136135
Method::new_modelled("listlockunspent", "ListLockUnspent", "list_lock_unspent"),
137136
Method::new_no_model("migratewallet", "MigrateWallet", "migrate_wallet"),
138-
Method::new_modelled("newkeypool", "NewKeyPool", "new_key_pool"),
137+
Method::new_nothing("newkeypool", "new_key_pool"),
139138
Method::new_modelled("psbtbumpfee", "PsbtBumpFee", "psbt_bump_fee"),
140139
Method::new_modelled(
141140
"listreceivedbyaddress",
@@ -152,7 +151,7 @@ pub const METHODS: &[Method] = &[
152151
Method::new_no_model("lockunspent", "LockUnspent", "lock_unspent"),
153152
Method::new_nothing("removeprunedfunds", "remove_pruned_funds"),
154153
Method::new_modelled("rescanblockchain", "RescanBlockchain", "rescan_blockchain"),
155-
Method::new_modelled("restorewallet", "RestoreWallet", "restore_wallet"),
154+
Method::new_no_model("restorewallet", "RestoreWallet", "restore_wallet"),
156155
Method::new_modelled("send", "Send", "send"),
157156
Method::new_modelled("sendall", "SendAll", "send_all"),
158157
Method::new_modelled("sendmany", "SendMany", "send_many"),
@@ -169,7 +168,7 @@ pub const METHODS: &[Method] = &[
169168
),
170169
Method::new_modelled("simulaterawtransaction", "SimulateRawTransaction", "simulate_raw_transaction"),
171170
Method::new_nothing("unloadwallet", "unload_wallet"),
172-
Method::new_no_model("upgradewallet", "UpgradeWalled", "upgrade_wallet"),
171+
Method::new_no_model("upgradewallet", "UpgradeWallet", "upgrade_wallet"),
173172
Method::new_modelled(
174173
"walletcreatefundedpsbt",
175174
"WalletCreateFundedPsbt",

0 commit comments

Comments
 (0)