From 073c63aa16cee36727dd271db45b683884de44d7 Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Mon, 7 Sep 2020 18:04:07 +0100 Subject: [PATCH] handle transaction inputs correctly (#512) * commit * bump grin crates to latest version on master --- Cargo.lock | 16 +++--- impls/src/node_clients/http.rs | 22 +++++++-- libwallet/src/api_impl/foreign.rs | 3 +- libwallet/src/api_impl/owner.rs | 4 +- libwallet/src/internal/tx.rs | 5 +- libwallet/src/slate.rs | 79 +++++++++++++++++------------- libwallet/src/slate_versions/v4.rs | 23 ++++++++- 7 files changed, 97 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8dd283404..8e343902d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1171,7 +1171,7 @@ checksum = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574" [[package]] name = "grin_api" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "bytes", "easy-jsonrpc-mw", @@ -1204,7 +1204,7 @@ dependencies = [ [[package]] name = "grin_chain" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "bit-vec", "bitflags 1.2.1", @@ -1228,7 +1228,7 @@ dependencies = [ [[package]] name = "grin_core" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "blake2-rfc", "byteorder", @@ -1254,7 +1254,7 @@ dependencies = [ [[package]] name = "grin_keychain" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "blake2-rfc", "byteorder", @@ -1276,7 +1276,7 @@ dependencies = [ [[package]] name = "grin_p2p" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "bitflags 1.2.1", "chrono", @@ -1297,7 +1297,7 @@ dependencies = [ [[package]] name = "grin_pool" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "blake2-rfc", "chrono", @@ -1331,7 +1331,7 @@ dependencies = [ [[package]] name = "grin_store" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "byteorder", "croaring-mw", @@ -1351,7 +1351,7 @@ dependencies = [ [[package]] name = "grin_util" version = "4.1.0-alpha.1" -source = "git+https://github.com/mimblewimble/grin#133089e9855686854c87d6fbe4d67df775918bee" +source = "git+https://github.com/mimblewimble/grin#7dc94576bd448bdca9f4f85eae9a150317b07976" dependencies = [ "backtrace", "base64 0.12.3", diff --git a/impls/src/node_clients/http.rs b/impls/src/node_clients/http.rs index 6d1bd76cf..e96d4c1d0 100644 --- a/impls/src/node_clients/http.rs +++ b/impls/src/node_clients/http.rs @@ -374,12 +374,14 @@ impl NodeClient for HTTPNodeClient { #[cfg(test)] mod tests { use super::*; - use crate::core::core::KernelFeatures; + use crate::core::core::{KernelFeatures, OutputFeatures, OutputIdentifier}; use crate::core::libtx::build; use crate::core::libtx::ProofBuilder; use crate::keychain::{ExtKeychain, Keychain}; - fn tx1i1o() -> Transaction { + // JSON api for "push_transaction" between wallet->node currently only supports "feature and commit" inputs. + // We will need to revisit this if we decide to support "commit only" inputs (no features) at wallet level. + fn tx1i1o_v2_compatible() -> Transaction { let keychain = ExtKeychain::from_random_seed(false).unwrap(); let builder = ProofBuilder::new(&keychain); let key_id1 = ExtKeychain::derive_key_id(1, 1, 0, 0, 0); @@ -391,14 +393,26 @@ mod tests { &builder, ) .unwrap(); - tx + + let inputs: Vec<_> = tx.inputs().into(); + let inputs: Vec<_> = inputs + .iter() + .map(|input| OutputIdentifier { + features: OutputFeatures::Plain, + commit: input.commitment(), + }) + .collect(); + Transaction { + body: tx.body.replace_inputs(inputs.as_slice().into()), + ..tx + } } // Wallet will "push" a transaction to node, serializing the transaction as json. // We are testing the json structure is what we expect here. #[test] fn test_transaction_json_ser_deser() { - let tx1 = tx1i1o(); + let tx1 = tx1i1o_v2_compatible(); let value = serde_json::to_value(&tx1).unwrap(); assert!(value["offset"].is_string()); diff --git a/libwallet/src/api_impl/foreign.rs b/libwallet/src/api_impl/foreign.rs index 92ad1eb2a..c707a74a3 100644 --- a/libwallet/src/api_impl/foreign.rs +++ b/libwallet/src/api_impl/foreign.rs @@ -17,7 +17,6 @@ use strum::IntoEnumIterator; use crate::api_impl::owner::finalize_tx as owner_finalize; use crate::api_impl::owner::{check_ttl, post_tx}; -use crate::grin_core::core::transaction::Transaction; use crate::grin_keychain::Keychain; use crate::grin_util::secp::key::SecretKey; use crate::internal::{selection, tx, updater}; @@ -91,7 +90,7 @@ where } } - ret_slate.tx = Some(Transaction::empty()); + ret_slate.tx = Some(Slate::empty_transaction()); let height = w.last_confirmed_height()?; let keychain = w.keychain(keychain_mask)?; diff --git a/libwallet/src/api_impl/owner.rs b/libwallet/src/api_impl/owner.rs index 5f5ce85dd..904a3221f 100644 --- a/libwallet/src/api_impl/owner.rs +++ b/libwallet/src/api_impl/owner.rs @@ -647,7 +647,7 @@ where } // if this is compact mode, we need to create the transaction now - ret_slate.tx = Some(Transaction::empty()); + ret_slate.tx = Some(Slate::empty_transaction()); // if self sending, make sure to store 'initiator' keys let context_res = w.get_private_context(keychain_mask, slate.id.as_bytes()); @@ -721,7 +721,7 @@ where let mut sl = slate.clone(); if sl.tx == None { - sl.tx = Some(Transaction::empty()); + sl.tx = Some(Slate::empty_transaction()); selection::repopulate_tx(&mut *w, keychain_mask, &mut sl, &context, true)?; } diff --git a/libwallet/src/internal/tx.rs b/libwallet/src/internal/tx.rs index f4c1f6451..7f18c5b76 100644 --- a/libwallet/src/internal/tx.rs +++ b/libwallet/src/internal/tx.rs @@ -585,7 +585,7 @@ mod test { use super::*; use rand::rngs::mock::StepRng; - use crate::grin_core::core::{Input, KernelFeatures}; + use crate::grin_core::core::KernelFeatures; use crate::grin_core::libtx::{build, ProofBuilder}; use crate::grin_keychain::{ BlindSum, BlindingFactor, ExtKeychain, ExtKeychainPath, Keychain, SwitchCommitmentType, @@ -615,8 +615,7 @@ mod test { ) .unwrap(); - let inputs: Vec = tx2.inputs().into(); - assert_eq!(tx1.outputs()[0].features(), inputs[0].features); + let inputs: Vec<_> = tx2.inputs().into(); assert_eq!(tx1.outputs()[0].commitment(), inputs[0].commitment()); } diff --git a/libwallet/src/slate.rs b/libwallet/src/slate.rs index 222748f28..2fe485ba5 100644 --- a/libwallet/src/slate.rs +++ b/libwallet/src/slate.rs @@ -18,8 +18,8 @@ use crate::error::{Error, ErrorKind}; use crate::grin_core::core::amount_to_hr_string; use crate::grin_core::core::transaction::{ - Input, KernelFeatures, NRDRelativeHeight, Output, OutputFeatures, Transaction, TxKernel, - Weighting, + Input, Inputs, KernelFeatures, NRDRelativeHeight, Output, OutputFeatures, Transaction, + TxKernel, Weighting, }; use crate::grin_core::core::verifier_cache::LruVerifierCache; use crate::grin_core::libtx::{aggsig, build, proof::ProofBuild, tx_fee}; @@ -242,6 +242,14 @@ impl Slate { Ok(()) } + /// Build a new empty transaction. + /// Wallet currently only supports tx with "features and commit" inputs. + pub fn empty_transaction() -> Transaction { + let mut tx = Transaction::empty(); + tx.body = tx.body.replace_inputs(Inputs::FeaturesAndCommit(vec![])); + tx + } + /// Create a new slate pub fn blank(num_participants: u8, is_invoice: bool) -> Slate { let np = match num_participants { @@ -256,7 +264,7 @@ impl Slate { num_participants: np, // assume 2 if not present id: Uuid::new_v4(), state, - tx: Some(Transaction::empty()), + tx: Some(Slate::empty_transaction()), amount: 0, fee: 0, ttl_cutoff_height: 0, @@ -816,35 +824,26 @@ impl From<&Slate> for SlateV4 { } } -// Node's Transaction object and lock height to SlateV4 `coms` impl From<&Slate> for Option> { - fn from(slate: &Slate) -> Option> { - let mut ret_vec = vec![]; - let (ins, outs) = match &slate.tx { - Some(t) => { - // TODO - input features are to be deprecated - // inputs here should be treated as a vec of commitments - // CommitsV4 should probably handle optional features. - let ins: Vec = t.inputs().into(); - (ins, t.outputs().to_vec()) + fn from(slate: &Slate) -> Self { + match slate.tx { + None => None, + Some(ref tx) => { + let mut ret_vec = vec![]; + match tx.inputs() { + Inputs::CommitOnly(_) => panic!("commit only inputs unsupported"), + Inputs::FeaturesAndCommit(ref inputs) => { + for input in inputs { + ret_vec.push(input.into()); + } + } + } + for output in tx.outputs() { + ret_vec.push(output.into()); + } + Some(ret_vec) } - None => return None, - }; - for i in ins.iter() { - ret_vec.push(CommitsV4 { - f: i.features.into(), - c: i.commit, - p: None, - }); - } - for o in outs.iter() { - ret_vec.push(CommitsV4 { - f: o.features().into(), - c: o.commitment(), - p: Some(o.proof), - }); } - Some(ret_vec) } } @@ -1017,20 +1016,30 @@ pub fn tx_from_slate_v4(slate: &SlateV4) -> Option { excess, excess_sig, }; - let mut tx = Transaction::empty().with_kernel(kernel); + let mut tx = Slate::empty_transaction().with_kernel(kernel); + + let mut outputs = vec![]; + let mut inputs = vec![]; for c in coms.iter() { - match c.p { - Some(p) => tx = tx.with_output(Output::new(c.f.into(), c.c, p)), + match &c.p { + Some(p) => { + outputs.push(Output::new(c.f.into(), c.c, p.clone())); + } None => { - tx = tx.with_input(Input { + inputs.push(Input { features: c.f.into(), commit: c.c, - }) + }); } } } - tx = tx.with_offset(slate.off.clone()); + + tx.body = tx + .body + .replace_inputs(inputs.as_slice().into()) + .replace_outputs(outputs.as_slice()); + tx.offset = slate.off.clone(); Some(tx) } diff --git a/libwallet/src/slate_versions/v4.rs b/libwallet/src/slate_versions/v4.rs index bfcffc937..0b09a629c 100644 --- a/libwallet/src/slate_versions/v4.rs +++ b/libwallet/src/slate_versions/v4.rs @@ -53,7 +53,7 @@ //! * The `receiver_signature` field is renamed to `rsig` //! * `rsig` may be omitted if it has not yet been filled out -use crate::grin_core::core::{Output, TxKernel}; +use crate::grin_core::core::{Input, Output, TxKernel}; use crate::grin_core::libtx::secp_ser; use crate::grin_keychain::{BlindingFactor, Identifier}; use crate::grin_util::secp; @@ -252,6 +252,27 @@ pub struct CommitsV4 { pub p: Option, } +impl From<&Output> for CommitsV4 { + fn from(out: &Output) -> CommitsV4 { + CommitsV4 { + f: out.features().into(), + c: out.commitment(), + p: Some(out.proof()), + } + } +} + +// This will need to be reworked once we no longer support input features with "commit only" inputs. +impl From<&Input> for CommitsV4 { + fn from(input: &Input) -> CommitsV4 { + CommitsV4 { + f: input.features.into(), + c: input.commitment(), + p: None, + } + } +} + fn default_output_feature() -> OutputFeaturesV4 { OutputFeaturesV4(0) }