From 53d77990d3698665a1f92d4fce57968c025feba6 Mon Sep 17 00:00:00 2001 From: Greg Martin Date: Fri, 31 Mar 2023 17:54:40 +0000 Subject: [PATCH 1/7] Add flags to `wallet inscribe` and `wallet send` to allow setting the postage sizes. --- src/subcommand/preview.rs | 1 + src/subcommand/wallet/inscribe.rs | 17 +++- src/subcommand/wallet/send.rs | 12 +++ src/subcommand/wallet/transaction_builder.rs | 82 ++++++++++++++++++-- 4 files changed, 105 insertions(+), 7 deletions(-) diff --git a/src/subcommand/preview.rs b/src/subcommand/preview.rs index d9f402e47e..5ab7173bc5 100644 --- a/src/subcommand/preview.rs +++ b/src/subcommand/preview.rs @@ -86,6 +86,7 @@ impl Preview { dry_run: false, no_limit: false, destination: None, + postage: Some(TransactionBuilder::DEFAULT_TARGET_POSTAGE), }, )), } diff --git a/src/subcommand/wallet/inscribe.rs b/src/subcommand/wallet/inscribe.rs index 1df5c15061..f8e53a9677 100644 --- a/src/subcommand/wallet/inscribe.rs +++ b/src/subcommand/wallet/inscribe.rs @@ -50,6 +50,8 @@ pub(crate) struct Inscribe { pub(crate) dry_run: bool, #[clap(long, help = "Send inscription to .")] pub(crate) destination: Option
, + #[clap(long, help = "Amount of postage to include in the inscription. Default `10000 sats`")] + pub(crate) postage: Option, } impl Inscribe { @@ -84,6 +86,10 @@ impl Inscribe { self.commit_fee_rate.unwrap_or(self.fee_rate), self.fee_rate, self.no_limit, + match self.postage { + Some(postage) => postage, + _ => TransactionBuilder::DEFAULT_TARGET_POSTAGE, + }, )?; utxos.insert( @@ -151,6 +157,7 @@ impl Inscribe { commit_fee_rate: FeeRate, reveal_fee_rate: FeeRate, no_limit: bool, + postage: Amount, ) -> Result<(Transaction, Transaction, TweakedKeyPair)> { let satpoint = if let Some(satpoint) = satpoint { satpoint @@ -223,7 +230,7 @@ impl Inscribe { commit_tx_address.clone(), change, commit_fee_rate, - reveal_fee + TransactionBuilder::TARGET_POSTAGE, + reveal_fee + postage, )?; let (vout, output) = unsigned_commit_tx @@ -389,6 +396,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .unwrap(); @@ -420,6 +428,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .unwrap(); @@ -455,6 +464,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .unwrap_err() .to_string(); @@ -497,6 +507,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .is_ok()) } @@ -533,6 +544,7 @@ mod tests { FeeRate::try_from(fee_rate).unwrap(), FeeRate::try_from(fee_rate).unwrap(), false, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .unwrap(); @@ -595,6 +607,7 @@ mod tests { FeeRate::try_from(commit_fee_rate).unwrap(), FeeRate::try_from(fee_rate).unwrap(), false, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .unwrap(); @@ -644,6 +657,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .unwrap_err() .to_string(); @@ -675,6 +689,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), true, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, ) .unwrap(); diff --git a/src/subcommand/wallet/send.rs b/src/subcommand/wallet/send.rs index 85a22f3c09..34e2209e37 100644 --- a/src/subcommand/wallet/send.rs +++ b/src/subcommand/wallet/send.rs @@ -6,6 +6,10 @@ pub(crate) struct Send { outgoing: Outgoing, #[clap(long, help = "Use fee rate of sats/vB")] fee_rate: FeeRate, + #[clap(long, help = "Target amount of postage to include in the sent output. Default `10000 sats`")] + pub(crate) target_postage: Option, + #[clap(long, help = "Maximum amount of postage to include in the sent output. Default `20000 sats`")] + pub(crate) max_postage: Option, } #[derive(Serialize, Deserialize)] @@ -78,6 +82,14 @@ impl Send { self.address, change, self.fee_rate, + match self.target_postage { + Some(target_postage) => target_postage, + _ => TransactionBuilder::DEFAULT_TARGET_POSTAGE, + }, + match self.max_postage { + Some(max_postage) => max_postage, + _ => TransactionBuilder::DEFAULT_MAX_POSTAGE, + }, )?; let signed_tx = client diff --git a/src/subcommand/wallet/transaction_builder.rs b/src/subcommand/wallet/transaction_builder.rs index 11f73d4f65..c2fac0a33b 100644 --- a/src/subcommand/wallet/transaction_builder.rs +++ b/src/subcommand/wallet/transaction_builder.rs @@ -107,6 +107,8 @@ pub struct TransactionBuilder { unused_change_addresses: Vec
, utxos: BTreeSet, target: Target, + target_postage: Amount, + max_postage: Amount, } type Result = std::result::Result; @@ -114,9 +116,9 @@ type Result = std::result::Result; impl TransactionBuilder { const ADDITIONAL_INPUT_VBYTES: usize = 58; const ADDITIONAL_OUTPUT_VBYTES: usize = 43; - const MAX_POSTAGE: Amount = Amount::from_sat(2 * 10_000); const SCHNORR_SIGNATURE_SIZE: usize = 64; - pub(crate) const TARGET_POSTAGE: Amount = Amount::from_sat(10_000); + pub(crate) const DEFAULT_MAX_POSTAGE: Amount = Amount::from_sat(2 * 10_000); + pub(crate) const DEFAULT_TARGET_POSTAGE: Amount = Amount::from_sat(10_000); pub fn build_transaction_with_postage( outgoing: SatPoint, @@ -125,6 +127,8 @@ impl TransactionBuilder { recipient: Address, change: [Address; 2], fee_rate: FeeRate, + target_postage: Amount, + max_postage: Amount, ) -> Result { Self::new( outgoing, @@ -134,6 +138,8 @@ impl TransactionBuilder { change, fee_rate, Target::Postage, + target_postage, + max_postage, )? .build_transaction() } @@ -164,6 +170,8 @@ impl TransactionBuilder { change, fee_rate, Target::Value(output_value), + Amount::from_sat(0), + Amount::from_sat(0), )? .build_transaction() } @@ -187,6 +195,8 @@ impl TransactionBuilder { change: [Address; 2], fee_rate: FeeRate, target: Target, + target_postage: Amount, + max_postage: Amount, ) -> Result { if change.contains(&recipient) { return Err(Error::DuplicateAddress(recipient)); @@ -208,6 +218,8 @@ impl TransactionBuilder { recipient, unused_change_addresses: change.to_vec(), target, + target_postage, + max_postage, }) } @@ -342,7 +354,7 @@ impl TransactionBuilder { if let Some(excess) = value.checked_sub(self.fee_rate.fee(self.estimate_vbytes())) { let (max, target) = match self.target { - Target::Postage => (Self::MAX_POSTAGE, Self::TARGET_POSTAGE), + Target::Postage => (self.max_postage, self.target_postage), Target::Value(value) => (value, value), }; @@ -554,7 +566,7 @@ impl TransactionBuilder { match self.target { Target::Postage => { assert!( - Amount::from_sat(output.value) <= Self::MAX_POSTAGE + slop, + Amount::from_sat(output.value) <= self.max_postage + slop, "invariant: excess postage is stripped" ); } @@ -681,6 +693,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -724,6 +738,8 @@ mod tests { (change(1), Amount::from_sat(1_724)), ], target: Target::Postage, + target_postage: TransactionBuilder::DEFAULT_TARGET_POSTAGE, + max_postage: TransactionBuilder::DEFAULT_MAX_POSTAGE, }; pretty_assert_eq!( @@ -752,6 +768,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .is_explicitly_rbf()) @@ -769,6 +787,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, @@ -792,6 +812,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -816,6 +838,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, @@ -838,6 +862,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Err(Error::NotEnoughCardinalUtxos), ) @@ -858,6 +884,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Err(Error::NotEnoughCardinalUtxos), ) @@ -878,6 +906,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, @@ -885,7 +915,7 @@ mod tests { input: vec![tx_in(outpoint(1)), tx_in(outpoint(2))], output: vec![ tx_out(4_950, change(1)), - tx_out(TransactionBuilder::TARGET_POSTAGE.to_sat(), recipient()), + tx_out(TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), recipient()), tx_out(14_831, change(0)), ], }) @@ -905,6 +935,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .build() @@ -924,6 +956,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .build() @@ -943,6 +977,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .build() @@ -962,6 +998,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -987,6 +1025,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1009,13 +1049,15 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, lock_time: PackedLockTime::ZERO, input: vec![tx_in(outpoint(1))], output: vec![ - tx_out(TransactionBuilder::TARGET_POSTAGE.to_sat(), recipient()), + tx_out(TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), recipient()), tx_out(989_870, change(1)) ], }) @@ -1035,6 +1077,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1055,6 +1099,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, @@ -1080,6 +1126,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, @@ -1103,6 +1151,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1131,6 +1181,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1157,6 +1209,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1180,6 +1234,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1213,6 +1269,8 @@ mod tests { (change(1), Amount::from_sat(1_774)), ], target: Target::Postage, + target_postage: TransactionBuilder::DEFAULT_TARGET_POSTAGE, + max_postage: TransactionBuilder::DEFAULT_MAX_POSTAGE, } .build() .unwrap(); @@ -1242,6 +1300,8 @@ mod tests { (change(0), Amount::from_sat(1_774)), ], target: Target::Postage, + target_postage: TransactionBuilder::DEFAULT_TARGET_POSTAGE, + max_postage: TransactionBuilder::DEFAULT_MAX_POSTAGE, } .build() .unwrap(); @@ -1262,6 +1322,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Err(Error::NotEnoughCardinalUtxos) ) @@ -1279,6 +1341,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Err(Error::UtxoContainsAdditionalInscription { outgoing_satpoint: satpoint(1, 0), @@ -1301,6 +1365,8 @@ mod tests { recipient(), [change(0), change(1)], fee_rate, + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap(); @@ -1486,6 +1552,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, @@ -1608,6 +1676,8 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(250.0).unwrap(), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ), Ok(Transaction { version: 1, From ff2b5cec9e945423272cf9614dd9185369887169 Mon Sep 17 00:00:00 2001 From: Greg Martin Date: Sun, 9 Apr 2023 21:44:01 +0000 Subject: [PATCH 2/7] Format --- src/subcommand/wallet/inscribe.rs | 5 ++++- src/subcommand/wallet/send.rs | 10 ++++++++-- src/subcommand/wallet/transaction_builder.rs | 10 ++++++++-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/subcommand/wallet/inscribe.rs b/src/subcommand/wallet/inscribe.rs index f8e53a9677..e686778172 100644 --- a/src/subcommand/wallet/inscribe.rs +++ b/src/subcommand/wallet/inscribe.rs @@ -50,7 +50,10 @@ pub(crate) struct Inscribe { pub(crate) dry_run: bool, #[clap(long, help = "Send inscription to .")] pub(crate) destination: Option
, - #[clap(long, help = "Amount of postage to include in the inscription. Default `10000 sats`")] + #[clap( + long, + help = "Amount of postage to include in the inscription. Default `10000 sats`" + )] pub(crate) postage: Option, } diff --git a/src/subcommand/wallet/send.rs b/src/subcommand/wallet/send.rs index 34e2209e37..658f7ce507 100644 --- a/src/subcommand/wallet/send.rs +++ b/src/subcommand/wallet/send.rs @@ -6,9 +6,15 @@ pub(crate) struct Send { outgoing: Outgoing, #[clap(long, help = "Use fee rate of sats/vB")] fee_rate: FeeRate, - #[clap(long, help = "Target amount of postage to include in the sent output. Default `10000 sats`")] + #[clap( + long, + help = "Target amount of postage to include in the sent output. Default `10000 sats`" + )] pub(crate) target_postage: Option, - #[clap(long, help = "Maximum amount of postage to include in the sent output. Default `20000 sats`")] + #[clap( + long, + help = "Maximum amount of postage to include in the sent output. Default `20000 sats`" + )] pub(crate) max_postage: Option, } diff --git a/src/subcommand/wallet/transaction_builder.rs b/src/subcommand/wallet/transaction_builder.rs index c2fac0a33b..aea6870b1b 100644 --- a/src/subcommand/wallet/transaction_builder.rs +++ b/src/subcommand/wallet/transaction_builder.rs @@ -915,7 +915,10 @@ mod tests { input: vec![tx_in(outpoint(1)), tx_in(outpoint(2))], output: vec![ tx_out(4_950, change(1)), - tx_out(TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), recipient()), + tx_out( + TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), + recipient() + ), tx_out(14_831, change(0)), ], }) @@ -1057,7 +1060,10 @@ mod tests { lock_time: PackedLockTime::ZERO, input: vec![tx_in(outpoint(1))], output: vec![ - tx_out(TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), recipient()), + tx_out( + TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), + recipient() + ), tx_out(989_870, change(1)) ], }) From 4d67321abf53b6fbdb637bb4a551d8972fb79878 Mon Sep 17 00:00:00 2001 From: raphjaph Date: Thu, 10 Aug 2023 13:43:57 +0200 Subject: [PATCH 3/7] quick fix --- src/subcommand/wallet/inscribe.rs | 3 --- src/subcommand/wallet/transaction_builder.rs | 6 ++++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/subcommand/wallet/inscribe.rs b/src/subcommand/wallet/inscribe.rs index c63d7d765f..b9f84bc445 100644 --- a/src/subcommand/wallet/inscribe.rs +++ b/src/subcommand/wallet/inscribe.rs @@ -51,14 +51,11 @@ pub(crate) struct Inscribe { pub(crate) dry_run: bool, #[clap(long, help = "Send inscription to .")] pub(crate) destination: Option>, - pub(crate) destination: Option
, #[clap( long, help = "Amount of postage to include in the inscription. Default `10000 sats`" )] pub(crate) postage: Option, - pub(crate) destination: Option>, - } impl Inscribe { diff --git a/src/subcommand/wallet/transaction_builder.rs b/src/subcommand/wallet/transaction_builder.rs index 8ce15f0214..7175bec6c7 100644 --- a/src/subcommand/wallet/transaction_builder.rs +++ b/src/subcommand/wallet/transaction_builder.rs @@ -1780,6 +1780,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1825,6 +1827,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1877,6 +1881,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), + TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap(); From 63f598fbfdee3823416b0b5a5af24450e4eaf88b Mon Sep 17 00:00:00 2001 From: raphjaph Date: Mon, 14 Aug 2023 14:10:20 +0200 Subject: [PATCH 4/7] quick fix --- src/subcommand/wallet/send.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/subcommand/wallet/send.rs b/src/subcommand/wallet/send.rs index 654c9c6d6a..392eb34fc0 100644 --- a/src/subcommand/wallet/send.rs +++ b/src/subcommand/wallet/send.rs @@ -77,6 +77,12 @@ impl Send { get_change_address(&client, &options)?, ]; + let (target_postage, max_postage) = if let Some(target_postage) = self.target_postage { + (target_postage, 2 * target_postage) + } else { + (TransactionBuilder::DEFAULT_TARGET_POSTAGE, TransactionBuilder::DEFAULT_MAX_POSTAGE) + }; + let unsigned_transaction = TransactionBuilder::build_transaction_with_postage( satpoint, inscriptions, @@ -84,14 +90,8 @@ impl Send { address, change, self.fee_rate, - match self.target_postage { - Some(target_postage) => target_postage, - _ => TransactionBuilder::DEFAULT_TARGET_POSTAGE, - }, - match self.max_postage { - Some(max_postage) => max_postage, - _ => TransactionBuilder::DEFAULT_MAX_POSTAGE, - }, + target_postage, + max_postage, )?; let signed_tx = client From c7aec719fcd09dd83f6d1b80d410ca69f130c054 Mon Sep 17 00:00:00 2001 From: raphjaph Date: Wed, 16 Aug 2023 16:40:00 +0200 Subject: [PATCH 5/7] before huge refactor --- src/subcommand/preview.rs | 2 +- src/subcommand/wallet.rs | 2 +- src/subcommand/wallet/inscribe.rs | 20 +- src/subcommand/wallet/send.rs | 20 +- src/subcommand/wallet/transaction_builder.rs | 224 +++++++++---------- tests/wallet/inscribe.rs | 23 ++ tests/wallet/send.rs | 36 +++ 7 files changed, 179 insertions(+), 148 deletions(-) diff --git a/src/subcommand/preview.rs b/src/subcommand/preview.rs index e55b3fad98..915fb136f2 100644 --- a/src/subcommand/preview.rs +++ b/src/subcommand/preview.rs @@ -87,7 +87,7 @@ impl Preview { dry_run: false, no_limit: false, destination: None, - postage: Some(TransactionBuilder::DEFAULT_TARGET_POSTAGE), + postage: Some(TransactionBuilder::TARGET_POSTAGE), }, )), } diff --git a/src/subcommand/wallet.rs b/src/subcommand/wallet.rs index 00c1d8da97..875ed5200f 100644 --- a/src/subcommand/wallet.rs +++ b/src/subcommand/wallet.rs @@ -24,7 +24,7 @@ pub mod receive; mod restore; pub mod sats; pub mod send; -pub(crate) mod transaction_builder; +pub mod transaction_builder; pub mod transactions; #[derive(Debug, Parser)] diff --git a/src/subcommand/wallet/inscribe.rs b/src/subcommand/wallet/inscribe.rs index b9f84bc445..6fc4782f6a 100644 --- a/src/subcommand/wallet/inscribe.rs +++ b/src/subcommand/wallet/inscribe.rs @@ -53,7 +53,7 @@ pub(crate) struct Inscribe { pub(crate) destination: Option>, #[clap( long, - help = "Amount of postage to include in the inscription. Default `10000 sats`" + help = "Amount of postage to include in the inscription. Default `10000sat`" )] pub(crate) postage: Option, } @@ -95,7 +95,7 @@ impl Inscribe { self.no_limit, match self.postage { Some(postage) => postage, - _ => TransactionBuilder::DEFAULT_TARGET_POSTAGE, + _ => TransactionBuilder::TARGET_POSTAGE, }, )?; @@ -403,7 +403,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .unwrap(); @@ -435,7 +435,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .unwrap(); @@ -471,7 +471,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .unwrap_err() .to_string(); @@ -514,7 +514,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .is_ok()) } @@ -551,7 +551,7 @@ mod tests { FeeRate::try_from(fee_rate).unwrap(), FeeRate::try_from(fee_rate).unwrap(), false, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .unwrap(); @@ -614,7 +614,7 @@ mod tests { FeeRate::try_from(commit_fee_rate).unwrap(), FeeRate::try_from(fee_rate).unwrap(), false, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .unwrap(); @@ -664,7 +664,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), false, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .unwrap_err() .to_string(); @@ -696,7 +696,7 @@ mod tests { FeeRate::try_from(1.0).unwrap(), FeeRate::try_from(1.0).unwrap(), true, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, + TransactionBuilder::TARGET_POSTAGE, ) .unwrap(); diff --git a/src/subcommand/wallet/send.rs b/src/subcommand/wallet/send.rs index 392eb34fc0..fb7f39eabe 100644 --- a/src/subcommand/wallet/send.rs +++ b/src/subcommand/wallet/send.rs @@ -1,4 +1,4 @@ -use {super::*, crate::wallet::Wallet}; +use {super::*, crate::subcommand::wallet::transaction_builder::Target, crate::wallet::Wallet}; #[derive(Debug, Parser)] pub(crate) struct Send { @@ -8,14 +8,9 @@ pub(crate) struct Send { fee_rate: FeeRate, #[clap( long, - help = "Target amount of postage to include in the sent output. Default `10000 sats`" + help = "Target amount of postage to include with sent inscriptions. Default `10000sat`" )] - pub(crate) target_postage: Option, - #[clap( - long, - help = "Maximum amount of postage to include in the sent output. Default `20000 sats`" - )] - pub(crate) max_postage: Option, + pub(crate) postage: Option, } #[derive(Serialize, Deserialize)] @@ -77,10 +72,10 @@ impl Send { get_change_address(&client, &options)?, ]; - let (target_postage, max_postage) = if let Some(target_postage) = self.target_postage { - (target_postage, 2 * target_postage) + let postage = if let Some(postage) = self.postage { + Target::ExactPostage(postage) } else { - (TransactionBuilder::DEFAULT_TARGET_POSTAGE, TransactionBuilder::DEFAULT_MAX_POSTAGE) + Target::Postage }; let unsigned_transaction = TransactionBuilder::build_transaction_with_postage( @@ -90,8 +85,7 @@ impl Send { address, change, self.fee_rate, - target_postage, - max_postage, + postage, )?; let signed_tx = client diff --git a/src/subcommand/wallet/transaction_builder.rs b/src/subcommand/wallet/transaction_builder.rs index 7175bec6c7..f31b5bfc22 100644 --- a/src/subcommand/wallet/transaction_builder.rs +++ b/src/subcommand/wallet/transaction_builder.rs @@ -63,9 +63,10 @@ pub enum Error { } #[derive(Debug, PartialEq)] -enum Target { +pub enum Target { Value(Amount), Postage, + ExactPostage(Amount), } impl fmt::Display for Error { @@ -110,8 +111,6 @@ pub struct TransactionBuilder { unused_change_addresses: Vec
, utxos: BTreeSet, target: Target, - target_postage: Amount, - max_postage: Amount, } type Result = std::result::Result; @@ -120,8 +119,40 @@ impl TransactionBuilder { const ADDITIONAL_INPUT_VBYTES: usize = 58; const ADDITIONAL_OUTPUT_VBYTES: usize = 43; const SCHNORR_SIGNATURE_SIZE: usize = 64; - pub(crate) const DEFAULT_MAX_POSTAGE: Amount = Amount::from_sat(2 * 10_000); - pub(crate) const DEFAULT_TARGET_POSTAGE: Amount = Amount::from_sat(10_000); + pub(crate) const TARGET_POSTAGE: Amount = Amount::from_sat(10_000); + pub(crate) const MAX_POSTAGE: Amount = Amount::from_sat(2 * 10_000); + + fn new( + outgoing: SatPoint, + inscriptions: BTreeMap, + amounts: BTreeMap, + recipient: Address, + change: [Address; 2], + fee_rate: FeeRate, + target: Target, + ) -> Result { + if change.contains(&recipient) { + return Err(Error::DuplicateAddress(recipient)); + } + + if change[0] == change[1] { + return Err(Error::DuplicateAddress(change[0].clone())); + } + + Ok(Self { + utxos: amounts.keys().cloned().collect(), + amounts, + change_addresses: change.iter().cloned().collect(), + fee_rate, + inputs: Vec::new(), + inscriptions, + outgoing, + outputs: Vec::new(), + recipient, + unused_change_addresses: change.to_vec(), + target, + }) + } pub fn build_transaction_with_postage( outgoing: SatPoint, @@ -130,8 +161,7 @@ impl TransactionBuilder { recipient: Address, change: [Address; 2], fee_rate: FeeRate, - target_postage: Amount, - max_postage: Amount, + target: Target, ) -> Result { Self::new( outgoing, @@ -140,9 +170,7 @@ impl TransactionBuilder { recipient, change, fee_rate, - Target::Postage, - target_postage, - max_postage, + target, )? .build_transaction() } @@ -173,8 +201,6 @@ impl TransactionBuilder { change, fee_rate, Target::Value(output_value), - Amount::from_sat(0), - Amount::from_sat(0), )? .build_transaction() } @@ -190,41 +216,6 @@ impl TransactionBuilder { .build() } - fn new( - outgoing: SatPoint, - inscriptions: BTreeMap, - amounts: BTreeMap, - recipient: Address, - change: [Address; 2], - fee_rate: FeeRate, - target: Target, - target_postage: Amount, - max_postage: Amount, - ) -> Result { - if change.contains(&recipient) { - return Err(Error::DuplicateAddress(recipient)); - } - - if change[0] == change[1] { - return Err(Error::DuplicateAddress(change[0].clone())); - } - - Ok(Self { - utxos: amounts.keys().cloned().collect(), - amounts, - change_addresses: change.iter().cloned().collect(), - fee_rate, - inputs: Vec::new(), - inscriptions, - outgoing, - outputs: Vec::new(), - recipient, - unused_change_addresses: change.to_vec(), - target, - target_postage, - max_postage, - }) - } fn select_outgoing(mut self) -> Result { for (inscribed_satpoint, inscription_id) in &self.inscriptions { @@ -326,7 +317,9 @@ impl TransactionBuilder { let estimated_fee = self.estimate_fee(); let min_value = match self.target { - Target::Postage => self.outputs.last().unwrap().0.script_pubkey().dust_value(), + Target::Postage | Target::ExactPostage(_) => { + self.outputs.last().unwrap().0.script_pubkey().dust_value() + } Target::Value(value) => value, }; @@ -384,7 +377,8 @@ impl TransactionBuilder { if let Some(excess) = value.checked_sub(self.fee_rate.fee(self.estimate_vbytes())) { let (max, target) = match self.target { - Target::Postage => (self.max_postage, self.target_postage), + Target::ExactPostage(postage) => (postage, postage), + Target::Postage => (Self::TARGET_POSTAGE, Self::MAX_POSTAGE), Target::Value(value) => (value, value), }; @@ -473,7 +467,7 @@ impl TransactionBuilder { previous_output: OutPoint::null(), script_sig: ScriptBuf::new(), sequence: Sequence::ENABLE_RBF_NO_LOCKTIME, - witness: Witness::from_slice(&[&[0; TransactionBuilder::SCHNORR_SIGNATURE_SIZE]]), + witness: Witness::from_slice(&[&[0; Self::SCHNORR_SIGNATURE_SIZE]]), }) .collect(), output: outputs @@ -596,7 +590,13 @@ impl TransactionBuilder { match self.target { Target::Postage => { assert!( - Amount::from_sat(output.value) <= self.max_postage + slop, + Amount::from_sat(output.value) <= Self::MAX_POSTAGE + slop, + "invariant: excess postage is stripped" + ); + } + Target::ExactPostage(postage) => { + assert!( + Amount::from_sat(output.value) <= postage + slop, "invariant: excess postage is stripped" ); } @@ -758,8 +758,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -803,8 +801,6 @@ mod tests { (change(1), Amount::from_sat(1_724)), ], target: Target::Postage, - target_postage: TransactionBuilder::DEFAULT_TARGET_POSTAGE, - max_postage: TransactionBuilder::DEFAULT_MAX_POSTAGE, }; pretty_assert_eq!( @@ -833,8 +829,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ) .unwrap() .is_explicitly_rbf()) @@ -852,8 +847,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, @@ -877,8 +871,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -903,8 +895,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, @@ -927,8 +918,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Err(Error::NotEnoughCardinalUtxos), ) @@ -949,8 +939,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage ), Err(Error::NotEnoughCardinalUtxos), ) @@ -971,8 +960,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, @@ -980,10 +968,7 @@ mod tests { input: vec![tx_in(outpoint(1)), tx_in(outpoint(2))], output: vec![ tx_out(4_950, change(1)), - tx_out( - TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), - recipient() - ), + tx_out(TransactionBuilder::TARGET_POSTAGE.to_sat(), recipient()), tx_out(14_831, change(0)), ], }) @@ -1003,8 +988,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .build() @@ -1024,8 +1007,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .build() @@ -1045,8 +1026,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .build() @@ -1066,8 +1045,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1094,8 +1071,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1118,18 +1093,14 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, input: vec![tx_in(outpoint(1))], output: vec![ - tx_out( - TransactionBuilder::DEFAULT_TARGET_POSTAGE.to_sat(), - recipient() - ), + tx_out(TransactionBuilder::TARGET_POSTAGE.to_sat(), recipient()), tx_out(989_870, change(1)) ], }) @@ -1149,8 +1120,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1171,8 +1140,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, @@ -1198,8 +1166,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, @@ -1223,8 +1190,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1253,8 +1218,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1281,8 +1244,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1306,8 +1267,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1341,8 +1300,6 @@ mod tests { (change(1), Amount::from_sat(1_774)), ], target: Target::Postage, - target_postage: TransactionBuilder::DEFAULT_TARGET_POSTAGE, - max_postage: TransactionBuilder::DEFAULT_MAX_POSTAGE, } .build() .unwrap(); @@ -1372,8 +1329,6 @@ mod tests { (change(0), Amount::from_sat(1_774)), ], target: Target::Postage, - target_postage: TransactionBuilder::DEFAULT_TARGET_POSTAGE, - max_postage: TransactionBuilder::DEFAULT_MAX_POSTAGE, } .build() .unwrap(); @@ -1394,8 +1349,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Err(Error::NotEnoughCardinalUtxos) ) @@ -1413,8 +1367,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Err(Error::UtxoContainsAdditionalInscription { outgoing_satpoint: satpoint(1, 0), @@ -1437,8 +1390,7 @@ mod tests { recipient(), [change(0), change(1)], fee_rate, - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ) .unwrap(); @@ -1625,8 +1577,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, @@ -1749,8 +1700,7 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(250.0).unwrap(), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, + Target::Postage, ), Ok(Transaction { version: 1, @@ -1780,8 +1730,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1827,8 +1775,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap() .select_outgoing() @@ -1881,8 +1827,6 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), - TransactionBuilder::DEFAULT_TARGET_POSTAGE, - TransactionBuilder::DEFAULT_MAX_POSTAGE, ) .unwrap(); @@ -1923,4 +1867,38 @@ mod tests { Amount::from_sat(20_000), ); } + + #[test] + fn build_transaction_with_custom_postage() { + let utxos = vec![(outpoint(1), Amount::from_sat(1_000_000))]; + + let fee_rate = FeeRate::try_from(17.3).unwrap(); + + let transaction = TransactionBuilder::build_transaction_with_postage( + satpoint(1, 0), + BTreeMap::from([(satpoint(1, 0), inscription_id(1))]), + utxos.into_iter().collect(), + recipient(), + [change(0), change(1)], + fee_rate, + Target::ExactPostage(Amount::from_sat(66_000)), + ) + .unwrap(); + + let fee = + fee_rate.fee(transaction.vsize() + TransactionBuilder::SCHNORR_SIGNATURE_SIZE / 4 + 1); + + pretty_assert_eq!( + transaction, + Transaction { + version: 1, + lock_time: LockTime::ZERO, + input: vec![tx_in(outpoint(1))], + output: vec![ + tx_out(66_000, recipient()), + tx_out(1_000_000 - 66_000 - fee.to_sat(), change(1)) + ], + } + ) + } } diff --git a/tests/wallet/inscribe.rs b/tests/wallet/inscribe.rs index 03a20ee03a..c545ecb2fe 100644 --- a/tests/wallet/inscribe.rs +++ b/tests/wallet/inscribe.rs @@ -417,3 +417,26 @@ fn inscribe_with_no_limit() { .write("degenerate.png", four_megger) .rpc_server(&rpc_server); } + +#[test] +fn inscribe_works_with_postage() { + let rpc_server = test_bitcoincore_rpc::spawn(); + create_wallet(&rpc_server); + rpc_server.mine_blocks(1); + + CommandBuilder::new(format!( + "wallet inscribe foo.txt --postage 5btc --fee-rate 10" + )) + .write("foo.txt", [0; 350]) + .rpc_server(&rpc_server) + .run_and_check_output::(); + + rpc_server.mine_blocks(1); + + let inscriptions = CommandBuilder::new(format!("wallet inscriptions")) + .write("foo.txt", [0; 350]) + .rpc_server(&rpc_server) + .run_and_check_output::>(); + + pretty_assert_eq!(inscriptions[0].postage, 5 * COIN_VALUE); +} diff --git a/tests/wallet/send.rs b/tests/wallet/send.rs index 5ecf6122c8..1100586fab 100644 --- a/tests/wallet/send.rs +++ b/tests/wallet/send.rs @@ -346,3 +346,39 @@ fn user_must_provide_fee_rate_to_send() { ) .run_and_extract_stdout(); } + +#[test] +fn wallet_send_with_fee_rate_and_target_postage() { + let rpc_server = test_bitcoincore_rpc::spawn(); + create_wallet(&rpc_server); + rpc_server.mine_blocks(1); + + let Inscribe { inscription, .. } = inscribe(&rpc_server); + + CommandBuilder::new(format!( + "wallet send bc1qw508d6qejxtdg4y5r3zarvary0c5xw7kv8f3t4 {inscription} --fee-rate 2.0 --postage 77000sat" + )) + .rpc_server(&rpc_server) + .stdout_regex("[[:xdigit:]]{64}\n") + .run_and_extract_stdout(); + + dbg!(&rpc_server.mempool()); + + let tx = &rpc_server.mempool()[0]; + let mut fee = 0; + for input in &tx.input { + fee += rpc_server + .get_utxo_amount(&input.previous_output) + .unwrap() + .to_sat(); + } + for output in &tx.output { + fee -= output.value; + } + + let fee_rate = fee as f64 / tx.vsize() as f64; + + pretty_assert_eq!(fee_rate, 2.0); + dbg!(&tx.output); + pretty_assert_eq!(tx.output[0].value, 77_000); +} From c4f4d2296e537d52bc9d4df63910b818e91e1a42 Mon Sep 17 00:00:00 2001 From: raphjaph Date: Wed, 16 Aug 2023 17:52:45 +0200 Subject: [PATCH 6/7] refactor --- src/subcommand/wallet/inscribe.rs | 9 +- src/subcommand/wallet/send.rs | 5 +- src/subcommand/wallet/transaction_builder.rs | 240 ++++++++++--------- tests/wallet/inscribe.rs | 12 +- tests/wallet/send.rs | 3 - 5 files changed, 136 insertions(+), 133 deletions(-) diff --git a/src/subcommand/wallet/inscribe.rs b/src/subcommand/wallet/inscribe.rs index 6fc4782f6a..95636ce4ea 100644 --- a/src/subcommand/wallet/inscribe.rs +++ b/src/subcommand/wallet/inscribe.rs @@ -1,6 +1,6 @@ use { super::*, - crate::wallet::Wallet, + crate::{subcommand::wallet::transaction_builder::Target, wallet::Wallet}, bitcoin::{ blockdata::{opcodes, script}, key::PrivateKey, @@ -230,15 +230,16 @@ impl Inscribe { &reveal_script, ); - let unsigned_commit_tx = TransactionBuilder::build_transaction_with_value( + let unsigned_commit_tx = TransactionBuilder::new( satpoint, inscriptions, utxos, commit_tx_address.clone(), change, commit_fee_rate, - reveal_fee + postage, - )?; + Target::Value(reveal_fee + postage), + )? + .build_transaction()?; let (vout, output) = unsigned_commit_tx .output diff --git a/src/subcommand/wallet/send.rs b/src/subcommand/wallet/send.rs index fb7f39eabe..0c808806eb 100644 --- a/src/subcommand/wallet/send.rs +++ b/src/subcommand/wallet/send.rs @@ -78,7 +78,7 @@ impl Send { Target::Postage }; - let unsigned_transaction = TransactionBuilder::build_transaction_with_postage( + let unsigned_transaction = TransactionBuilder::new( satpoint, inscriptions, unspent_outputs, @@ -86,7 +86,8 @@ impl Send { change, self.fee_rate, postage, - )?; + )? + .build_transaction()?; let signed_tx = client .sign_raw_transaction_with_wallet(&unsigned_transaction, None, None)? diff --git a/src/subcommand/wallet/transaction_builder.rs b/src/subcommand/wallet/transaction_builder.rs index f31b5bfc22..91910f618b 100644 --- a/src/subcommand/wallet/transaction_builder.rs +++ b/src/subcommand/wallet/transaction_builder.rs @@ -98,7 +98,7 @@ impl fmt::Display for Error { impl std::error::Error for Error {} -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct TransactionBuilder { amounts: BTreeMap, change_addresses: BTreeSet
, @@ -122,7 +122,7 @@ impl TransactionBuilder { pub(crate) const TARGET_POSTAGE: Amount = Amount::from_sat(10_000); pub(crate) const MAX_POSTAGE: Amount = Amount::from_sat(2 * 10_000); - fn new( + pub(crate) fn new( outgoing: SatPoint, inscriptions: BTreeMap, amounts: BTreeMap, @@ -139,6 +139,20 @@ impl TransactionBuilder { return Err(Error::DuplicateAddress(change[0].clone())); } + match target { + Target::Value(output_value) | Target::ExactPostage(output_value) => { + let dust_value = recipient.script_pubkey().dust_value(); + + if output_value < dust_value { + return Err(Error::Dust { + output_value, + dust_value, + }); + } + } + _ => (), + } + Ok(Self { utxos: amounts.keys().cloned().collect(), amounts, @@ -154,58 +168,7 @@ impl TransactionBuilder { }) } - pub fn build_transaction_with_postage( - outgoing: SatPoint, - inscriptions: BTreeMap, - amounts: BTreeMap, - recipient: Address, - change: [Address; 2], - fee_rate: FeeRate, - target: Target, - ) -> Result { - Self::new( - outgoing, - inscriptions, - amounts, - recipient, - change, - fee_rate, - target, - )? - .build_transaction() - } - - pub fn build_transaction_with_value( - outgoing: SatPoint, - inscriptions: BTreeMap, - amounts: BTreeMap, - recipient: Address, - change: [Address; 2], - fee_rate: FeeRate, - output_value: Amount, - ) -> Result { - let dust_value = recipient.script_pubkey().dust_value(); - - if output_value < dust_value { - return Err(Error::Dust { - output_value, - dust_value, - }); - } - - Self::new( - outgoing, - inscriptions, - amounts, - recipient, - change, - fee_rate, - Target::Value(output_value), - )? - .build_transaction() - } - - fn build_transaction(self) -> Result { + pub(crate) fn build_transaction(self) -> Result { self .select_outgoing()? .align_outgoing() @@ -216,7 +179,6 @@ impl TransactionBuilder { .build() } - fn select_outgoing(mut self) -> Result { for (inscribed_satpoint, inscription_id) in &self.inscriptions { if self.outgoing.outpoint == inscribed_satpoint.outpoint @@ -317,10 +279,8 @@ impl TransactionBuilder { let estimated_fee = self.estimate_fee(); let min_value = match self.target { - Target::Postage | Target::ExactPostage(_) => { - self.outputs.last().unwrap().0.script_pubkey().dust_value() - } - Target::Value(value) => value, + Target::Postage => self.outputs.last().unwrap().0.script_pubkey().dust_value(), + Target::Value(value) | Target::ExactPostage(value) => value, }; let total = min_value @@ -378,7 +338,7 @@ impl TransactionBuilder { if let Some(excess) = value.checked_sub(self.fee_rate.fee(self.estimate_vbytes())) { let (max, target) = match self.target { Target::ExactPostage(postage) => (postage, postage), - Target::Postage => (Self::TARGET_POSTAGE, Self::MAX_POSTAGE), + Target::Postage => (Self::MAX_POSTAGE, Self::TARGET_POSTAGE), Target::Value(value) => (value, value), }; @@ -822,7 +782,7 @@ mod tests { fn transactions_are_rbf() { let utxos = vec![(outpoint(1), Amount::from_sat(5_000))]; - assert!(TransactionBuilder::build_transaction_with_postage( + assert!(TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), utxos.into_iter().collect(), @@ -832,6 +792,8 @@ mod tests { Target::Postage, ) .unwrap() + .build_transaction() + .unwrap() .is_explicitly_rbf()) } @@ -840,7 +802,7 @@ mod tests { let utxos = vec![(outpoint(1), Amount::from_sat(5_000))]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), utxos.into_iter().collect(), @@ -848,7 +810,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -888,7 +852,7 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 4_950), BTreeMap::new(), utxos.into_iter().collect(), @@ -896,7 +860,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -911,7 +877,7 @@ mod tests { let utxos = vec![(outpoint(1), Amount::from_sat(5_000))]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 4_950), BTreeMap::new(), utxos.into_iter().collect(), @@ -919,7 +885,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) } @@ -932,7 +900,7 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 4_950), BTreeMap::new(), utxos.into_iter().collect(), @@ -940,7 +908,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage - ), + ) + .unwrap() + .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) } @@ -953,7 +923,7 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 4_950), BTreeMap::new(), utxos.into_iter().collect(), @@ -961,7 +931,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1086,7 +1058,7 @@ mod tests { let utxos = vec![(outpoint(1), Amount::from_sat(1_000_000))]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), utxos.into_iter().collect(), @@ -1094,7 +1066,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1133,7 +1107,7 @@ mod tests { let utxos = vec![(outpoint(1), Amount::from_sat(10_000))]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 3_333), BTreeMap::new(), utxos.into_iter().collect(), @@ -1141,7 +1115,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1159,7 +1135,7 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 1), BTreeMap::new(), utxos.into_iter().collect(), @@ -1167,7 +1143,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1342,7 +1320,7 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::from([(satpoint(2, 10 * COIN_VALUE), inscription_id(1))]), utxos.into_iter().collect(), @@ -1350,7 +1328,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Err(Error::NotEnoughCardinalUtxos) ) } @@ -1360,7 +1340,7 @@ mod tests { let utxos = vec![(outpoint(1), Amount::from_sat(1_000))]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::from([(satpoint(1, 500), inscription_id(1))]), utxos.into_iter().collect(), @@ -1368,7 +1348,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Err(Error::UtxoContainsAdditionalInscription { outgoing_satpoint: satpoint(1, 0), inscribed_satpoint: satpoint(1, 500), @@ -1383,7 +1365,7 @@ mod tests { let fee_rate = FeeRate::try_from(17.3).unwrap(); - let transaction = TransactionBuilder::build_transaction_with_postage( + let transaction = TransactionBuilder::new( satpoint(1, 0), BTreeMap::from([(satpoint(1, 0), inscription_id(1))]), utxos.into_iter().collect(), @@ -1392,6 +1374,8 @@ mod tests { fee_rate, Target::Postage, ) + .unwrap() + .build_transaction() .unwrap(); let fee = @@ -1413,15 +1397,17 @@ mod tests { let utxos = vec![(outpoint(1), Amount::from_sat(5_000))]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), utxos.into_iter().collect(), recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - Amount::from_sat(1000) - ), + Target::Value(Amount::from_sat(1000)) + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1439,15 +1425,17 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), utxos.into_iter().collect(), recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - Amount::from_sat(1500) - ), + Target::Value(Amount::from_sat(1500)) + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1462,14 +1450,14 @@ mod tests { let utxos = vec![(outpoint(1), Amount::from_sat(1_000))]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::from([(satpoint(1, 500), inscription_id(1))]), utxos.into_iter().collect(), recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - Amount::from_sat(1) + Target::Value(Amount::from_sat(1)) ), Err(Error::Dust { output_value: Amount::from_sat(1), @@ -1486,15 +1474,17 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), utxos.into_iter().collect(), recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - Amount::from_sat(1000) - ), + Target::Value(Amount::from_sat(1000)) + ) + .unwrap() + .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) } @@ -1507,15 +1497,17 @@ mod tests { ]; pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), utxos.into_iter().collect(), recipient(), [change(0), change(1)], FeeRate::try_from(4.0).unwrap(), - Amount::from_sat(1000) - ), + Target::Value(Amount::from_sat(1000)) + ) + .unwrap() + .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) } @@ -1545,7 +1537,7 @@ mod tests { #[test] fn do_not_strip_excess_value_if_it_would_create_dust() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(1_000))] @@ -1554,8 +1546,10 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), - Amount::from_sat(707) - ), + Target::Value(Amount::from_sat(707)) + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1568,7 +1562,7 @@ mod tests { #[test] fn possible_to_create_output_of_exactly_max_postage() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(20_099))] @@ -1578,7 +1572,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1591,7 +1587,7 @@ mod tests { #[test] fn do_not_strip_excess_value_if_additional_output_cannot_pay_fee() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(1_500))] @@ -1600,8 +1596,10 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(5.0).unwrap(), - Amount::from_sat(1000) - ), + Target::Value(Amount::from_sat(1000)) + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1614,7 +1612,7 @@ mod tests { #[test] fn correct_error_is_returned_when_fee_cannot_be_paid() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(1_500))] @@ -1623,8 +1621,10 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(6.0).unwrap(), - Amount::from_sat(1000) - ), + Target::Value(Amount::from_sat(1000)) + ) + .unwrap() + .build_transaction(), Err(Error::NotEnoughCardinalUtxos) ); } @@ -1632,7 +1632,7 @@ mod tests { #[test] fn recipient_address_must_be_unique() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(1000))] @@ -1641,7 +1641,7 @@ mod tests { recipient(), [recipient(), change(1)], FeeRate::try_from(0.0).unwrap(), - Amount::from_sat(1000) + Target::Value(Amount::from_sat(1000)) ), Err(Error::DuplicateAddress(recipient())) ); @@ -1650,7 +1650,7 @@ mod tests { #[test] fn change_addresses_must_be_unique() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(1000))] @@ -1659,7 +1659,7 @@ mod tests { recipient(), [change(0), change(0)], FeeRate::try_from(0.0).unwrap(), - Amount::from_sat(1000) + Target::Value(Amount::from_sat(1000)) ), Err(Error::DuplicateAddress(change(0))) ); @@ -1668,7 +1668,7 @@ mod tests { #[test] fn output_over_value_because_fees_prevent_excess_value_stripping() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_value( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(2000))] @@ -1677,8 +1677,10 @@ mod tests { recipient(), [change(0), change(1)], FeeRate::try_from(2.0).unwrap(), - Amount::from_sat(1500) - ), + Target::Value(Amount::from_sat(1500)) + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1691,7 +1693,7 @@ mod tests { #[test] fn output_over_max_postage_because_fees_prevent_excess_value_stripping() { pretty_assert_eq!( - TransactionBuilder::build_transaction_with_postage( + TransactionBuilder::new( satpoint(1, 0), BTreeMap::new(), vec![(outpoint(1), Amount::from_sat(45000))] @@ -1701,7 +1703,9 @@ mod tests { [change(0), change(1)], FeeRate::try_from(250.0).unwrap(), Target::Postage, - ), + ) + .unwrap() + .build_transaction(), Ok(Transaction { version: 1, lock_time: LockTime::ZERO, @@ -1874,7 +1878,7 @@ mod tests { let fee_rate = FeeRate::try_from(17.3).unwrap(); - let transaction = TransactionBuilder::build_transaction_with_postage( + let transaction = TransactionBuilder::new( satpoint(1, 0), BTreeMap::from([(satpoint(1, 0), inscription_id(1))]), utxos.into_iter().collect(), @@ -1883,6 +1887,8 @@ mod tests { fee_rate, Target::ExactPostage(Amount::from_sat(66_000)), ) + .unwrap() + .build_transaction() .unwrap(); let fee = diff --git a/tests/wallet/inscribe.rs b/tests/wallet/inscribe.rs index c545ecb2fe..47527a8284 100644 --- a/tests/wallet/inscribe.rs +++ b/tests/wallet/inscribe.rs @@ -424,16 +424,14 @@ fn inscribe_works_with_postage() { create_wallet(&rpc_server); rpc_server.mine_blocks(1); - CommandBuilder::new(format!( - "wallet inscribe foo.txt --postage 5btc --fee-rate 10" - )) - .write("foo.txt", [0; 350]) - .rpc_server(&rpc_server) - .run_and_check_output::(); + CommandBuilder::new("wallet inscribe foo.txt --postage 5btc --fee-rate 10".to_string()) + .write("foo.txt", [0; 350]) + .rpc_server(&rpc_server) + .run_and_check_output::(); rpc_server.mine_blocks(1); - let inscriptions = CommandBuilder::new(format!("wallet inscriptions")) + let inscriptions = CommandBuilder::new("wallet inscriptions".to_string()) .write("foo.txt", [0; 350]) .rpc_server(&rpc_server) .run_and_check_output::>(); diff --git a/tests/wallet/send.rs b/tests/wallet/send.rs index 1100586fab..0e26079877 100644 --- a/tests/wallet/send.rs +++ b/tests/wallet/send.rs @@ -362,8 +362,6 @@ fn wallet_send_with_fee_rate_and_target_postage() { .stdout_regex("[[:xdigit:]]{64}\n") .run_and_extract_stdout(); - dbg!(&rpc_server.mempool()); - let tx = &rpc_server.mempool()[0]; let mut fee = 0; for input in &tx.input { @@ -379,6 +377,5 @@ fn wallet_send_with_fee_rate_and_target_postage() { let fee_rate = fee as f64 / tx.vsize() as f64; pretty_assert_eq!(fee_rate, 2.0); - dbg!(&tx.output); pretty_assert_eq!(tx.output[0].value, 77_000); } From a08adbbb4e1f1175b8ecfc86f0a51858cfcdc3bb Mon Sep 17 00:00:00 2001 From: raphjaph Date: Thu, 17 Aug 2023 13:09:15 +0200 Subject: [PATCH 7/7] constructor returns no result --- src/subcommand/wallet/inscribe.rs | 2 +- src/subcommand/wallet/send.rs | 2 +- src/subcommand/wallet/transaction_builder.rs | 97 +++++++------------- 3 files changed, 34 insertions(+), 67 deletions(-) diff --git a/src/subcommand/wallet/inscribe.rs b/src/subcommand/wallet/inscribe.rs index 95636ce4ea..f712890656 100644 --- a/src/subcommand/wallet/inscribe.rs +++ b/src/subcommand/wallet/inscribe.rs @@ -238,7 +238,7 @@ impl Inscribe { change, commit_fee_rate, Target::Value(reveal_fee + postage), - )? + ) .build_transaction()?; let (vout, output) = unsigned_commit_tx diff --git a/src/subcommand/wallet/send.rs b/src/subcommand/wallet/send.rs index 0c808806eb..2c6c9fd574 100644 --- a/src/subcommand/wallet/send.rs +++ b/src/subcommand/wallet/send.rs @@ -86,7 +86,7 @@ impl Send { change, self.fee_rate, postage, - )? + ) .build_transaction()?; let signed_tx = client diff --git a/src/subcommand/wallet/transaction_builder.rs b/src/subcommand/wallet/transaction_builder.rs index 91910f618b..77003bc53a 100644 --- a/src/subcommand/wallet/transaction_builder.rs +++ b/src/subcommand/wallet/transaction_builder.rs @@ -130,18 +130,36 @@ impl TransactionBuilder { change: [Address; 2], fee_rate: FeeRate, target: Target, - ) -> Result { - if change.contains(&recipient) { - return Err(Error::DuplicateAddress(recipient)); + ) -> Self { + Self { + utxos: amounts.keys().cloned().collect(), + amounts, + change_addresses: change.iter().cloned().collect(), + fee_rate, + inputs: Vec::new(), + inscriptions, + outgoing, + outputs: Vec::new(), + recipient, + unused_change_addresses: change.to_vec(), + target, + } + } + + pub(crate) fn build_transaction(self) -> Result { + if self.change_addresses.len() < 2 { + return Err(Error::DuplicateAddress( + self.change_addresses.first().unwrap().clone(), + )); } - if change[0] == change[1] { - return Err(Error::DuplicateAddress(change[0].clone())); + if self.change_addresses.contains(&self.recipient) { + return Err(Error::DuplicateAddress(self.recipient)); } - match target { + match self.target { Target::Value(output_value) | Target::ExactPostage(output_value) => { - let dust_value = recipient.script_pubkey().dust_value(); + let dust_value = self.recipient.script_pubkey().dust_value(); if output_value < dust_value { return Err(Error::Dust { @@ -153,22 +171,6 @@ impl TransactionBuilder { _ => (), } - Ok(Self { - utxos: amounts.keys().cloned().collect(), - amounts, - change_addresses: change.iter().cloned().collect(), - fee_rate, - inputs: Vec::new(), - inscriptions, - outgoing, - outputs: Vec::new(), - recipient, - unused_change_addresses: change.to_vec(), - target, - }) - } - - pub(crate) fn build_transaction(self) -> Result { self .select_outgoing()? .align_outgoing() @@ -719,7 +721,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap(); @@ -791,7 +792,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction() .unwrap() .is_explicitly_rbf()) @@ -811,7 +811,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -836,7 +835,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap() .align_outgoing() @@ -861,7 +859,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -886,7 +883,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) @@ -909,7 +905,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage ) - .unwrap() .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) @@ -932,7 +927,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -961,7 +955,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build() .unwrap(); } @@ -980,7 +973,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build() .unwrap(); } @@ -999,7 +991,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build() .unwrap(); } @@ -1018,7 +1009,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap(); @@ -1044,7 +1034,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap(); @@ -1067,7 +1056,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1095,7 +1083,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap() .build() @@ -1116,7 +1103,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1144,7 +1130,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1169,7 +1154,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap() .align_outgoing() @@ -1197,7 +1181,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap() .align_outgoing() @@ -1223,7 +1206,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap() .strip_value() @@ -1246,7 +1228,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .select_outgoing() .unwrap() .strip_value() @@ -1329,7 +1310,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Err(Error::NotEnoughCardinalUtxos) ) @@ -1349,7 +1329,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Err(Error::UtxoContainsAdditionalInscription { outgoing_satpoint: satpoint(1, 0), @@ -1374,7 +1353,6 @@ mod tests { fee_rate, Target::Postage, ) - .unwrap() .build_transaction() .unwrap(); @@ -1406,7 +1384,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(1000)) ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1434,7 +1411,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(1500)) ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1458,7 +1434,8 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(1)) - ), + ) + .build_transaction(), Err(Error::Dust { output_value: Amount::from_sat(1), dust_value: Amount::from_sat(294) @@ -1483,7 +1460,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(1000)) ) - .unwrap() .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) @@ -1506,7 +1482,6 @@ mod tests { FeeRate::try_from(4.0).unwrap(), Target::Value(Amount::from_sat(1000)) ) - .unwrap() .build_transaction(), Err(Error::NotEnoughCardinalUtxos), ) @@ -1548,7 +1523,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(707)) ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1573,7 +1547,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1598,7 +1571,6 @@ mod tests { FeeRate::try_from(5.0).unwrap(), Target::Value(Amount::from_sat(1000)) ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1623,7 +1595,6 @@ mod tests { FeeRate::try_from(6.0).unwrap(), Target::Value(Amount::from_sat(1000)) ) - .unwrap() .build_transaction(), Err(Error::NotEnoughCardinalUtxos) ); @@ -1642,7 +1613,8 @@ mod tests { [recipient(), change(1)], FeeRate::try_from(0.0).unwrap(), Target::Value(Amount::from_sat(1000)) - ), + ) + .build_transaction(), Err(Error::DuplicateAddress(recipient())) ); } @@ -1660,7 +1632,8 @@ mod tests { [change(0), change(0)], FeeRate::try_from(0.0).unwrap(), Target::Value(Amount::from_sat(1000)) - ), + ) + .build_transaction(), Err(Error::DuplicateAddress(change(0))) ); } @@ -1679,7 +1652,6 @@ mod tests { FeeRate::try_from(2.0).unwrap(), Target::Value(Amount::from_sat(1500)) ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1704,7 +1676,6 @@ mod tests { FeeRate::try_from(250.0).unwrap(), Target::Postage, ) - .unwrap() .build_transaction(), Ok(Transaction { version: 1, @@ -1735,7 +1706,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), ) - .unwrap() .select_outgoing() .unwrap() .add_value() @@ -1780,7 +1750,6 @@ mod tests { FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), ) - .unwrap() .select_outgoing() .unwrap() .align_outgoing() @@ -1831,8 +1800,7 @@ mod tests { [change(0), change(1)], FeeRate::try_from(1.0).unwrap(), Target::Value(Amount::from_sat(10_000)), - ) - .unwrap(); + ); assert_eq!( tx_builder @@ -1887,7 +1855,6 @@ mod tests { fee_rate, Target::ExactPostage(Amount::from_sat(66_000)), ) - .unwrap() .build_transaction() .unwrap();