From d2429bf5eedc101709e20b67c26bc525b013bfc7 Mon Sep 17 00:00:00 2001 From: raphjaph Date: Sun, 19 Nov 2023 19:12:29 -0300 Subject: [PATCH 1/7] Add destination field to Batch --- batch.yaml | 3 +++ src/subcommand/wallet/inscribe.rs | 12 ++---------- src/subcommand/wallet/inscribe/batch.rs | 18 ++++++++++++++++-- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/batch.yaml b/batch.yaml index ded5e7879a..6bb182543c 100644 --- a/batch.yaml +++ b/batch.yaml @@ -25,10 +25,13 @@ inscriptions: metus est et odio. Nullam venenatis, urna et molestie vestibulum, orci mi efficitur risus, eu malesuada diam lorem sed velit. Nam fermentum dolor et luctus euismod. + destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4 - file: token.json metaprotocol: brc-20 + destination: bc1pxwww0ct9ue7e8tdnlmug5m2tamfn7q06sahstg39ys4c9f3340qqxrdu9k - file: tulip.png metadata: author: Satoshi Nakamoto + destination: bc1pdqrcrxa8vx6gy75mfdfj84puhxffh4fq46h3gkp6jxdd0vjcsdyspfxcv6 diff --git a/src/subcommand/wallet/inscribe.rs b/src/subcommand/wallet/inscribe.rs index 1efbe1f63a..233ecc65c0 100644 --- a/src/subcommand/wallet/inscribe.rs +++ b/src/subcommand/wallet/inscribe.rs @@ -153,7 +153,8 @@ impl Inscribe { parent_info = Inscribe::get_parent_info(batchfile.parent, &index, &utxos, &client, chain)?; - inscriptions = batchfile.inscriptions( + (inscriptions, destinations) = batchfile.inscriptions( + &client, chain, parent_info.as_ref().map(|info| info.tx_out.value), metadata, @@ -162,15 +163,6 @@ impl Inscribe { )?; mode = batchfile.mode; - - let destination_count = match batchfile.mode { - Mode::SharedOutput => 1, - Mode::SeparateOutputs => inscriptions.len(), - }; - - destinations = (0..destination_count) - .map(|_| get_change_address(&client, chain)) - .collect::>>()?; } _ => unreachable!(), } diff --git a/src/subcommand/wallet/inscribe/batch.rs b/src/subcommand/wallet/inscribe/batch.rs index 9a7444eaf8..0f5244f019 100644 --- a/src/subcommand/wallet/inscribe/batch.rs +++ b/src/subcommand/wallet/inscribe/batch.rs @@ -532,6 +532,7 @@ pub(crate) struct BatchEntry { pub(crate) file: PathBuf, pub(crate) metadata: Option, pub(crate) metaprotocol: Option, + pub(crate) destination: Option>, } impl BatchEntry { @@ -568,12 +569,13 @@ impl Batchfile { pub(crate) fn inscriptions( &self, + client: &Client, chain: Chain, parent_value: Option, metadata: Option>, postage: Amount, compress: bool, - ) -> Result> { + ) -> Result<(Vec, Vec
)> { assert!(!self.inscriptions.is_empty()); if metadata.is_some() { @@ -586,6 +588,7 @@ impl Batchfile { let mut pointer = parent_value.unwrap_or_default(); let mut inscriptions = Vec::new(); + let mut destinations = Vec::new(); for (i, entry) in self.inscriptions.iter().enumerate() { inscriptions.push(Inscription::from_file( chain, @@ -600,9 +603,20 @@ impl Batchfile { compress, )?); + if !(self.mode == Mode::SharedOutput && i >= 1) { + destinations.push(entry.destination.as_ref().map_or_else( + || get_change_address(client, chain), + |address| { + address + .clone() + .require_network(chain.network()) + .map_err(|e| e.into()) + }, + )?); + } pointer += postage.to_sat(); } - Ok(inscriptions) + Ok((inscriptions, destinations)) } } From 459f6f7fed50bfffa864d9a781871209a874783d Mon Sep 17 00:00:00 2001 From: raphjaph Date: Sun, 19 Nov 2023 19:48:38 -0300 Subject: [PATCH 2/7] Some tests --- src/subcommand/wallet/inscribe/batch.rs | 8 +++ tests/wallet/inscribe.rs | 83 +++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/src/subcommand/wallet/inscribe/batch.rs b/src/subcommand/wallet/inscribe/batch.rs index 0f5244f019..578f5c833c 100644 --- a/src/subcommand/wallet/inscribe/batch.rs +++ b/src/subcommand/wallet/inscribe/batch.rs @@ -577,6 +577,14 @@ impl Batchfile { compress: bool, ) -> Result<(Vec, Vec
)> { assert!(!self.inscriptions.is_empty()); + assert!( + !(self + .inscriptions + .iter() + .any(|entry| entry.destination.is_some()) + && self.mode == Mode::SharedOutput), + "invariant: destination field cannot be used in shared-output mode" + ); if metadata.is_some() { assert!(self diff --git a/tests/wallet/inscribe.rs b/tests/wallet/inscribe.rs index 0020bde3e0..dbb175b9aa 100644 --- a/tests/wallet/inscribe.rs +++ b/tests/wallet/inscribe.rs @@ -1343,3 +1343,86 @@ fn inscriptions_are_not_compressed_if_no_space_is_saved_by_compression() { assert_eq!(response.status(), StatusCode::OK); assert_eq!(response.text().unwrap(), "foo"); } + +#[test] +fn batch_inscribe_fails_if_invalid_destination_address() { + let rpc_server = test_bitcoincore_rpc::spawn(); + rpc_server.mine_blocks(1); + + assert_eq!(rpc_server.descriptors().len(), 0); + + create_wallet(&rpc_server); + + CommandBuilder::new("wallet inscribe --fee-rate 2.1 --batch batch.yaml") + .write("inscription.txt", "Hello World") + .write("batch.yaml", "mode: separate-outputs\ninscriptions:\n- file: inscription.txt\n destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t") + .rpc_server(&rpc_server) + .stderr_regex(".*bech32 address encoding error.*") + .expected_exit_code(1) + .run_and_extract_stdout(); +} + +#[test] +#[should_panic(expected = "invariant: destination field cannot be used in shared-output mode")] +fn batch_inscribe_fails_with_shared_output_and_destination_set() { + let rpc_server = test_bitcoincore_rpc::spawn(); + rpc_server.mine_blocks(1); + + assert_eq!(rpc_server.descriptors().len(), 0); + + create_wallet(&rpc_server); + + CommandBuilder::new("wallet inscribe --fee-rate 2.1 --batch batch.yaml") + .write("inscription.txt", "Hello World") + .write("tulip.png", "") + .write("batch.yaml", "mode: shared-output\ninscriptions:\n- file: inscription.txt\n destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4\n- file: tulip.png") + .rpc_server(&rpc_server) + .expected_exit_code(1) + .run_and_extract_stdout(); +} + +#[test] +fn batch_inscribe_works_with_some_destinations_set_and_others_not() { + let rpc_server = test_bitcoincore_rpc::spawn(); + rpc_server.mine_blocks(1); + + assert_eq!(rpc_server.descriptors().len(), 0); + + create_wallet(&rpc_server); + + let output = CommandBuilder::new("wallet inscribe --batch batch.yaml --fee-rate 55") + .write("inscription.txt", "Hello World") + .write("tulip.png", [0; 555]) + .write("meow.wav", [0; 2048]) + .write( + "batch.yaml", + "mode: separate-outputs\ninscriptions:\n- file: inscription.txt\n destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4\n- file: tulip.png\n- file: meow.wav\n destination: bc1pxwww0ct9ue7e8tdnlmug5m2tamfn7q06sahstg39ys4c9f3340qqxrdu9k\n" + ) + .rpc_server(&rpc_server) + .run_and_deserialize_output::(); + + rpc_server.mine_blocks(1); + + assert_eq!(rpc_server.descriptors().len(), 3); + + TestServer::spawn_with_args(&rpc_server, &[]).assert_response_regex( + format!("/inscription/{}", output.inscriptions[0].id), + ".* +
address
+
bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4
.*", + ); + + TestServer::spawn_with_args(&rpc_server, &[]).assert_response_regex( + format!("/inscription/{}", output.inscriptions[1].id), + ".* +
address
+
.*
.*", + ); + + TestServer::spawn_with_args(&rpc_server, &[]).assert_response_regex( + format!("/inscription/{}", output.inscriptions[2].id), + ".* +
address
+
bc1pxwww0ct9ue7e8tdnlmug5m2tamfn7q06sahstg39ys4c9f3340qqxrdu9k
.*", + ); +} From 934c4ca11f41e9a9c22251bd5f50b4adb31ce384 Mon Sep 17 00:00:00 2001 From: raphjaph Date: Mon, 20 Nov 2023 19:56:26 -0300 Subject: [PATCH 3/7] address some code review comments --- batch.yaml | 1 - src/subcommand/wallet/inscribe/batch.rs | 21 +++++++++-------- test-bitcoincore-rpc/src/lib.rs | 6 ++++- test-bitcoincore-rpc/src/server.rs | 1 + test-bitcoincore-rpc/src/state.rs | 2 ++ tests/wallet/inscribe.rs | 30 ++++++++++++++++--------- 6 files changed, 39 insertions(+), 22 deletions(-) diff --git a/batch.yaml b/batch.yaml index 06a9bf7bf4..ffe29ac596 100644 --- a/batch.yaml +++ b/batch.yaml @@ -32,7 +32,6 @@ inscriptions: - file: token.json metaprotocol: brc-20 - destination: bc1pxwww0ct9ue7e8tdnlmug5m2tamfn7q06sahstg39ys4c9f3340qqxrdu9k - file: tulip.png metadata: diff --git a/src/subcommand/wallet/inscribe/batch.rs b/src/subcommand/wallet/inscribe/batch.rs index 86ceb34394..1c07691498 100644 --- a/src/subcommand/wallet/inscribe/batch.rs +++ b/src/subcommand/wallet/inscribe/batch.rs @@ -530,10 +530,10 @@ pub(crate) enum Mode { #[derive(Deserialize, Default, PartialEq, Debug, Clone)] #[serde(deny_unknown_fields)] pub(crate) struct BatchEntry { + pub(crate) destination: Option>, pub(crate) file: PathBuf, pub(crate) metadata: Option, pub(crate) metaprotocol: Option, - pub(crate) destination: Option>, } impl BatchEntry { @@ -579,14 +579,17 @@ impl Batchfile { compress: bool, ) -> Result<(Vec, Vec
)> { assert!(!self.inscriptions.is_empty()); - assert!( - !(self - .inscriptions - .iter() - .any(|entry| entry.destination.is_some()) - && self.mode == Mode::SharedOutput), - "invariant: destination field cannot be used in shared-output mode" - ); + + if self + .inscriptions + .iter() + .any(|entry| entry.destination.is_some()) + && self.mode == Mode::SharedOutput + { + return Err(anyhow!( + "destination field cannot be used in shared-output mode" + )); + } if metadata.is_some() { assert!(self diff --git a/test-bitcoincore-rpc/src/lib.rs b/test-bitcoincore-rpc/src/lib.rs index b29cc95d98..3a10171547 100644 --- a/test-bitcoincore-rpc/src/lib.rs +++ b/test-bitcoincore-rpc/src/lib.rs @@ -31,7 +31,7 @@ use { server::Server, state::State, std::{ - collections::{BTreeMap, BTreeSet, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap, HashSet}, sync::{Arc, Mutex, MutexGuard}, thread, time::Duration, @@ -242,6 +242,10 @@ impl Handle { pub fn loaded_wallets(&self) -> BTreeSet { self.state().loaded_wallets.clone() } + + pub fn get_change_addresses(&self) -> Vec
{ + self.state().change_addresses.clone() + } } impl Drop for Handle { diff --git a/test-bitcoincore-rpc/src/server.rs b/test-bitcoincore-rpc/src/server.rs index 96d8ae83aa..006f04b028 100644 --- a/test-bitcoincore-rpc/src/server.rs +++ b/test-bitcoincore-rpc/src/server.rs @@ -479,6 +479,7 @@ impl Api for Server { let key_pair = KeyPair::new(&secp256k1, &mut rand::thread_rng()); let (public_key, _parity) = XOnlyPublicKey::from_keypair(&key_pair); let address = Address::p2tr(&secp256k1, public_key, None, self.network); + self.state().change_addresses.push(address.clone()); Ok(address) } diff --git a/test-bitcoincore-rpc/src/state.rs b/test-bitcoincore-rpc/src/state.rs index b2d09b0110..870794c471 100644 --- a/test-bitcoincore-rpc/src/state.rs +++ b/test-bitcoincore-rpc/src/state.rs @@ -2,6 +2,7 @@ use super::*; pub(crate) struct State { pub(crate) blocks: BTreeMap, + pub(crate) change_addresses: Vec
, pub(crate) descriptors: Vec, pub(crate) fail_lock_unspent: bool, pub(crate) hashes: Vec, @@ -29,6 +30,7 @@ impl State { Self { blocks, + change_addresses: Vec::new(), descriptors: Vec::new(), fail_lock_unspent, hashes, diff --git a/tests/wallet/inscribe.rs b/tests/wallet/inscribe.rs index 77347decb8..cbd5d543ad 100644 --- a/tests/wallet/inscribe.rs +++ b/tests/wallet/inscribe.rs @@ -1345,25 +1345,27 @@ fn inscriptions_are_not_compressed_if_no_space_is_saved_by_compression() { } #[test] -fn batch_inscribe_fails_if_invalid_destination_address() { - let rpc_server = test_bitcoincore_rpc::spawn(); +fn batch_inscribe_fails_if_invalid_network_destination_address() { + let rpc_server = test_bitcoincore_rpc::builder() + .network(Network::Regtest) + .build(); + rpc_server.mine_blocks(1); assert_eq!(rpc_server.descriptors().len(), 0); create_wallet(&rpc_server); - CommandBuilder::new("wallet inscribe --fee-rate 2.1 --batch batch.yaml") + CommandBuilder::new("--regtest wallet inscribe --fee-rate 2.1 --batch batch.yaml") .write("inscription.txt", "Hello World") - .write("batch.yaml", "mode: separate-outputs\ninscriptions:\n- file: inscription.txt\n destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t") + .write("batch.yaml", "mode: separate-outputs\ninscriptions:\n- file: inscription.txt\n destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4") .rpc_server(&rpc_server) - .stderr_regex(".*bech32 address encoding error.*") + .stderr_regex("error: address bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4 belongs to network bitcoin which is different from required regtest\n") .expected_exit_code(1) .run_and_extract_stdout(); } #[test] -#[should_panic(expected = "invariant: destination field cannot be used in shared-output mode")] fn batch_inscribe_fails_with_shared_output_and_destination_set() { let rpc_server = test_bitcoincore_rpc::spawn(); rpc_server.mine_blocks(1); @@ -1378,6 +1380,7 @@ fn batch_inscribe_fails_with_shared_output_and_destination_set() { .write("batch.yaml", "mode: shared-output\ninscriptions:\n- file: inscription.txt\n destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4\n- file: tulip.png") .rpc_server(&rpc_server) .expected_exit_code(1) + .stderr_regex("error: destination field cannot be used in shared-output mode\n") .run_and_extract_stdout(); } @@ -1405,21 +1408,26 @@ fn batch_inscribe_works_with_some_destinations_set_and_others_not() { assert_eq!(rpc_server.descriptors().len(), 3); - TestServer::spawn_with_args(&rpc_server, &[]).assert_response_regex( + let ord_server = TestServer::spawn_with_args(&rpc_server, &[]); + + ord_server.assert_response_regex( format!("/inscription/{}", output.inscriptions[0].id), ".*
address
bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4
.*", ); - TestServer::spawn_with_args(&rpc_server, &[]).assert_response_regex( + ord_server.assert_response_regex( format!("/inscription/{}", output.inscriptions[1].id), - ".* + format!( + ".*
address
-
.*
.*", +
{}
.*", + rpc_server.get_change_addresses()[0] + ), ); - TestServer::spawn_with_args(&rpc_server, &[]).assert_response_regex( + ord_server.assert_response_regex( format!("/inscription/{}", output.inscriptions[2].id), ".*
address
From b42c42948a69d32cc9c28d81957ead1949e8873b Mon Sep 17 00:00:00 2001 From: raphjaph Date: Mon, 20 Nov 2023 22:21:08 -0300 Subject: [PATCH 4/7] remove hash set --- test-bitcoincore-rpc/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-bitcoincore-rpc/src/lib.rs b/test-bitcoincore-rpc/src/lib.rs index 3a10171547..56c50e45a8 100644 --- a/test-bitcoincore-rpc/src/lib.rs +++ b/test-bitcoincore-rpc/src/lib.rs @@ -31,7 +31,7 @@ use { server::Server, state::State, std::{ - collections::{BTreeMap, BTreeSet, HashMap, HashSet}, + collections::{BTreeMap, BTreeSet, HashMap}, sync::{Arc, Mutex, MutexGuard}, thread, time::Duration, From 1088851e0654320324c3fdb61f94423e1e1ed125 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Mon, 20 Nov 2023 17:43:26 -0800 Subject: [PATCH 5/7] Update src/subcommand/wallet/inscribe/batch.rs --- src/subcommand/wallet/inscribe/batch.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/subcommand/wallet/inscribe/batch.rs b/src/subcommand/wallet/inscribe/batch.rs index 1c07691498..8e8679a806 100644 --- a/src/subcommand/wallet/inscribe/batch.rs +++ b/src/subcommand/wallet/inscribe/batch.rs @@ -587,7 +587,7 @@ impl Batchfile { && self.mode == Mode::SharedOutput { return Err(anyhow!( - "destination field cannot be used in shared-output mode" + "individual inscription destinations cannot be set in shared-output mode" )); } From 9847dc1d568854bfabad8ec762f08fad77bc2a2e Mon Sep 17 00:00:00 2001 From: raphjaph Date: Mon, 20 Nov 2023 23:12:48 -0300 Subject: [PATCH 6/7] clean up logic --- src/subcommand/wallet/inscribe/batch.rs | 31 +++++++++++++++---------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/subcommand/wallet/inscribe/batch.rs b/src/subcommand/wallet/inscribe/batch.rs index 1c07691498..66d8f5570f 100644 --- a/src/subcommand/wallet/inscribe/batch.rs +++ b/src/subcommand/wallet/inscribe/batch.rs @@ -601,7 +601,6 @@ impl Batchfile { let mut pointer = parent_value.unwrap_or_default(); let mut inscriptions = Vec::new(); - let mut destinations = Vec::new(); for (i, entry) in self.inscriptions.iter().enumerate() { inscriptions.push(Inscription::from_file( chain, @@ -616,20 +615,28 @@ impl Batchfile { compress, )?); - if !(self.mode == Mode::SharedOutput && i >= 1) { - destinations.push(entry.destination.as_ref().map_or_else( - || get_change_address(client, chain), - |address| { - address - .clone() - .require_network(chain.network()) - .map_err(|e| e.into()) - }, - )?); - } pointer += postage.to_sat(); } + let destinations = match self.mode { + Mode::SharedOutput => vec![get_change_address(client, chain)?], + Mode::SeparateOutputs => self + .inscriptions + .iter() + .map(|entry| { + entry.destination.as_ref().map_or_else( + || get_change_address(client, chain), + |address| { + address + .clone() + .require_network(chain.network()) + .map_err(|e| e.into()) + }, + ) + }) + .collect::, _>>()?, + }; + Ok((inscriptions, destinations)) } } From 77ec46e37842980e7a79fb3e8f2057b8b975b216 Mon Sep 17 00:00:00 2001 From: raphjaph Date: Mon, 20 Nov 2023 23:17:41 -0300 Subject: [PATCH 7/7] quick fix --- tests/wallet/inscribe.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/wallet/inscribe.rs b/tests/wallet/inscribe.rs index cbd5d543ad..3f189ffb1b 100644 --- a/tests/wallet/inscribe.rs +++ b/tests/wallet/inscribe.rs @@ -1380,7 +1380,7 @@ fn batch_inscribe_fails_with_shared_output_and_destination_set() { .write("batch.yaml", "mode: shared-output\ninscriptions:\n- file: inscription.txt\n destination: bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4\n- file: tulip.png") .rpc_server(&rpc_server) .expected_exit_code(1) - .stderr_regex("error: destination field cannot be used in shared-output mode\n") + .stderr_regex("error: individual inscription destinations cannot be set in shared-output mode\n") .run_and_extract_stdout(); }