Skip to content

Commit

Permalink
Change check_repair + certain functions to lock more granularly (mimb…
Browse files Browse the repository at this point in the history
…lewimble#252)

* Rename check-repair -> scan, make lock logic more granular

* rustfmt

* update owner api implementations where to allow granular locking where required

* rustfmt

* store init state on startup to determine whether full utxo scan is required for new wallets

* rustfmt

* fix for init status persist

* add start height argument to scan

* rustfmt
  • Loading branch information
yeastplume authored Nov 6, 2019
1 parent ec84a7c commit 30a5a38
Show file tree
Hide file tree
Showing 19 changed files with 414 additions and 931 deletions.
5 changes: 2 additions & 3 deletions api/src/foreign_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,10 +691,9 @@ pub fn run_doctest_foreign(
false,
);
//update local outputs after each block, so transaction IDs stay consistent
let mut w_lock = wallet1.lock();
let w = w_lock.lc_provider().unwrap().wallet_inst().unwrap();
let (wallet_refreshed, _) =
api_impl::owner::retrieve_summary_info(&mut **w, (&mask1).as_ref(), true, 1).unwrap();
api_impl::owner::retrieve_summary_info(wallet1.clone(), (&mask1).as_ref(), true, 1)
.unwrap();
assert!(wallet_refreshed);
}

Expand Down
89 changes: 24 additions & 65 deletions api/src/owner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,8 @@ where
refresh_from_node: bool,
tx_id: Option<u32>,
) -> Result<(bool, Vec<OutputCommitMapping>), Error> {
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
owner::retrieve_outputs(
&mut **w,
self.wallet_inst.clone(),
keychain_mask,
include_spent,
refresh_from_node,
Expand Down Expand Up @@ -402,10 +400,8 @@ where
tx_id: Option<u32>,
tx_slate_id: Option<Uuid>,
) -> Result<(bool, Vec<TxLogEntry>), Error> {
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
let mut res = owner::retrieve_txs(
&mut **w,
self.wallet_inst.clone(),
keychain_mask,
refresh_from_node,
tx_id,
Expand Down Expand Up @@ -468,10 +464,8 @@ where
refresh_from_node: bool,
minimum_confirmations: u64,
) -> Result<(bool, WalletInfo), Error> {
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
owner::retrieve_summary_info(
&mut **w,
self.wallet_inst.clone(),
keychain_mask,
refresh_from_node,
minimum_confirmations,
Expand Down Expand Up @@ -970,9 +964,7 @@ where
tx_id: Option<u32>,
tx_slate_id: Option<Uuid>,
) -> Result<(), Error> {
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
owner::cancel_tx(&mut **w, keychain_mask, tx_id, tx_slate_id)
owner::cancel_tx(self.wallet_inst.clone(), keychain_mask, tx_id, tx_slate_id)
}

/// Retrieves the stored transaction associated with a TxLogEntry. Can be used even after the
Expand Down Expand Up @@ -1085,48 +1077,6 @@ where
owner::verify_slate_messages(slate)
}

/// Scans the entire UTXO set from the node, creating outputs for each scanned
/// output that matches the wallet's master seed. This function is intended to be called as part
/// of a recovery process (either from BIP32 phrase or backup seed files,) and will error if the
/// wallet is non-empty, i.e. contains any outputs at all.
///
/// This operation scans the entire chain, and is expected to be time intensive. It is imperative
/// that no other processes should be trying to use the wallet at the same time this function is
/// running.
///
/// A single [TxLogEntry](../grin_wallet_libwallet/types/struct.TxLogEntry.html) is created for
/// all non-coinbase outputs discovered and restored during this process. A separate entry
/// is created for each coinbase output.
///
/// # Arguments
///
/// * `keychain_mask` - Wallet secret mask to XOR against the stored wallet seed before using, if
/// being used.
///
/// # Returns
/// * `Ok(())` if successful
/// * or [`libwallet::Error`](../grin_wallet_libwallet/struct.Error.html) if an error is encountered.

/// # Example
/// Set up as in [`new`](struct.Owner.html#method.new) method above.
/// ```
/// # grin_wallet_api::doctest_helper_setup_doc_env!(wallet, wallet_config);
///
/// let mut api_owner = Owner::new(wallet.clone());
/// let result = api_owner.restore(None);
///
/// if let Ok(_) = result {
/// // Wallet outputs should be consistent with what's on chain
/// // ...
/// }
/// ```
pub fn restore(&self, keychain_mask: Option<&SecretKey>) -> Result<(), Error> {
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
let res = owner::restore(&mut **w, keychain_mask);
res
}

/// Scans the entire UTXO set from the node, identify which outputs belong to the given wallet
/// update the wallet state to be consistent with what's currently in the UTXO set.
///
Expand All @@ -1145,7 +1095,9 @@ where
///
/// * `keychain_mask` - Wallet secret mask to XOR against the stored wallet seed before using, if
/// being used.
/// * `delete_unconfirmed` - if `false`, the check_repair process will be non-destructive, and
/// * `start_height` - If provided, the height of the first block from which to start scanning.
/// The scan will start from block 1 if this is not provided.
/// * `delete_unconfirmed` - if `false`, the scan process will be non-destructive, and
/// mostly limited to restoring missing outputs. It will leave unconfirmed transaction logs entries
/// and unconfirmed outputs intact. If `true`, the process will unlock all locked outputs,
/// restore all missing outputs, and mark any outputs that have been marked 'Spent' but are still
Expand All @@ -1165,8 +1117,9 @@ where
/// # grin_wallet_api::doctest_helper_setup_doc_env!(wallet, wallet_config);
///
/// let mut api_owner = Owner::new(wallet.clone());
/// let result = api_owner.check_repair(
/// let result = api_owner.scan(
/// None,
/// Some(20000),
/// false,
/// );
///
Expand All @@ -1176,14 +1129,18 @@ where
/// }
/// ```

pub fn check_repair(
pub fn scan(
&self,
keychain_mask: Option<&SecretKey>,
start_height: Option<u64>,
delete_unconfirmed: bool,
) -> Result<(), Error> {
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
owner::check_repair(&mut **w, keychain_mask, delete_unconfirmed)
owner::scan(
self.wallet_inst.clone(),
keychain_mask,
start_height,
delete_unconfirmed,
)
}

/// Retrieves the last known height known by the wallet. This is determined as follows:
Expand Down Expand Up @@ -1228,11 +1185,13 @@ where
&self,
keychain_mask: Option<&SecretKey>,
) -> Result<NodeHeightResult, Error> {
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
// Test keychain mask, to keep API consistent
let _ = w.keychain(keychain_mask)?;
let mut res = owner::node_height(&mut **w, keychain_mask)?;
{
let mut w_lock = self.wallet_inst.lock();
let w = w_lock.lc_provider()?.wallet_inst()?;
// Test keychain mask, to keep API consistent
let _ = w.keychain(keychain_mask)?;
}
let mut res = owner::node_height(self.wallet_inst.clone(), keychain_mask)?;
if self.doctest_mode {
// return a consistent hash for doctest
res.header_hash =
Expand Down
50 changes: 8 additions & 42 deletions api/src/owner_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1161,16 +1161,16 @@ pub trait OwnerRpc: Sync + Send {
fn verify_slate_messages(&self, slate: VersionedSlate) -> Result<(), ErrorKind>;

/**
Networked version of [Owner::restore](struct.Owner.html#method.restore).
Networked version of [Owner::scan](struct.Owner.html#method.scan).
```
# grin_wallet_api::doctest_helper_json_rpc_owner_assert_response!(
# r#"
{
"jsonrpc": "2.0",
"method": "restore",
"params": [],
"method": "scan",
"params": [null, false],
"id": 1
}
# "#
Expand All @@ -1187,36 +1187,7 @@ pub trait OwnerRpc: Sync + Send {
# , false, 1, false, false, false);
```
*/
fn restore(&self) -> Result<(), ErrorKind>;

/**
Networked version of [Owner::check_repair](struct.Owner.html#method.check_repair).
```
# grin_wallet_api::doctest_helper_json_rpc_owner_assert_response!(
# r#"
{
"jsonrpc": "2.0",
"method": "check_repair",
"params": [false],
"id": 1
}
# "#
# ,
# r#"
{
"id": 1,
"jsonrpc": "2.0",
"result": {
"Ok": null
}
}
# "#
# , false, 1, false, false, false);
```
*/
fn check_repair(&self, delete_unconfirmed: bool) -> Result<(), ErrorKind>;
fn scan(&self, start_height: Option<u64>, delete_unconfirmed: bool) -> Result<(), ErrorKind>;

/**
Networked version of [Owner::node_height](struct.Owner.html#method.node_height).
Expand Down Expand Up @@ -1355,12 +1326,8 @@ where
Owner::verify_slate_messages(self, None, &Slate::from(slate)).map_err(|e| e.kind())
}

fn restore(&self) -> Result<(), ErrorKind> {
Owner::restore(self, None).map_err(|e| e.kind())
}

fn check_repair(&self, delete_unconfirmed: bool) -> Result<(), ErrorKind> {
Owner::check_repair(self, None, delete_unconfirmed).map_err(|e| e.kind())
fn scan(&self, start_height: Option<u64>, delete_unconfirmed: bool) -> Result<(), ErrorKind> {
Owner::scan(self, None, start_height, delete_unconfirmed).map_err(|e| e.kind())
}

fn node_height(&self) -> Result<NodeHeightResult, ErrorKind> {
Expand Down Expand Up @@ -1491,10 +1458,9 @@ pub fn run_doctest_owner(
false,
);
//update local outputs after each block, so transaction IDs stay consistent
let mut w_lock = wallet1.lock();
let w = w_lock.lc_provider().unwrap().wallet_inst().unwrap();
let (wallet_refreshed, _) =
api_impl::owner::retrieve_summary_info(&mut **w, (&mask1).as_ref(), true, 1).unwrap();
api_impl::owner::retrieve_summary_info(wallet1.clone(), (&mask1).as_ref(), true, 1)
.unwrap();
assert!(wallet_refreshed);
}

Expand Down
63 changes: 22 additions & 41 deletions api/src/owner_rpc_s.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1220,48 +1220,18 @@ pub trait OwnerRpcS {
fn verify_slate_messages(&self, token: Token, slate: VersionedSlate) -> Result<(), ErrorKind>;

/**
Networked version of [Owner::restore](struct.Owner.html#method.restore).
Networked version of [Owner::scan](struct.Owner.html#method.scan).
```
# grin_wallet_api::doctest_helper_json_rpc_owner_assert_response!(
# r#"
{
"jsonrpc": "2.0",
"method": "restore",
"params": {
"token": "d202964900000000d302964900000000d402964900000000d502964900000000"
},
"id": 1
}
# "#
# ,
# r#"
{
"id": 1,
"jsonrpc": "2.0",
"result": {
"Ok": null
}
}
# "#
# , true, 1, false, false, false);
```
*/
fn restore(&self, token: Token) -> Result<(), ErrorKind>;

/**
Networked version of [Owner::check_repair](struct.Owner.html#method.check_repair).
```
# grin_wallet_api::doctest_helper_json_rpc_owner_assert_response!(
# r#"
{
"jsonrpc": "2.0",
"method": "check_repair",
"method": "scan",
"params": {
"token": "d202964900000000d302964900000000d402964900000000d502964900000000",
"start_height": 1,
"delete_unconfirmed": false
},
"id": 1
Expand All @@ -1280,7 +1250,12 @@ pub trait OwnerRpcS {
# , true, 1, false, false, false);
```
*/
fn check_repair(&self, token: Token, delete_unconfirmed: bool) -> Result<(), ErrorKind>;
fn scan(
&self,
token: Token,
start_height: Option<u64>,
delete_unconfirmed: bool,
) -> Result<(), ErrorKind>;

/**
Networked version of [Owner::node_height](struct.Owner.html#method.node_height).
Expand Down Expand Up @@ -1867,13 +1842,19 @@ where
.map_err(|e| e.kind())
}

fn restore(&self, token: Token) -> Result<(), ErrorKind> {
Owner::restore(self, (&token.keychain_mask).as_ref()).map_err(|e| e.kind())
}

fn check_repair(&self, token: Token, delete_unconfirmed: bool) -> Result<(), ErrorKind> {
Owner::check_repair(self, (&token.keychain_mask).as_ref(), delete_unconfirmed)
.map_err(|e| e.kind())
fn scan(
&self,
token: Token,
start_height: Option<u64>,
delete_unconfirmed: bool,
) -> Result<(), ErrorKind> {
Owner::scan(
self,
(&token.keychain_mask).as_ref(),
start_height,
delete_unconfirmed,
)
.map_err(|e| e.kind())
}

fn node_height(&self, token: Token) -> Result<NodeHeightResult, ErrorKind> {
Expand Down
Loading

0 comments on commit 30a5a38

Please sign in to comment.