zec
/
librustzcash
mirror of https://github.com/zcash/librustzcash.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #932 from zcash/update_chain_tip-bugs 

Fix `update_chain_tip` bugs
This commit is contained in:
str4d 2023-09-05 16:31:39 +01:00 committed by GitHub
parent 224e021558 f132af1f33
commit 4e823d92eb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 446 additions and 103 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

549
zcash_client_sqlite/src/wallet/scanning.rs
View File

@ -18,6 +18,8 @@ use crate::{
PRUNING_DEPTH, VERIFY_LOOKAHEAD,
};
use super::wallet_birthday;
#[derive(Debug, Clone, Copy)]
enum InsertOn {
Left,
@ -715,8 +717,17 @@ pub(crate) fn update_chain_tip<P: consensus::Parameters>(
params: &P,
new_tip: BlockHeight,
) -> Result<(), SqliteClientError> {
// If the caller provided a chain tip that is before Sapling activation, do nothing.
let sapling_activation = match params.activation_height(NetworkUpgrade::Sapling) {
Some(h) if h <= new_tip => h,
_ => return Ok(()),
};
// Read the previous max scanned height from the blocks table
let prior_tip = block_height_extrema(conn)?.map(|(_, prior_tip)| prior_tip);
let max_scanned = block_height_extrema(conn)?.map(|(_, max_scanned)| max_scanned);
// Read the wallet birthday (if known).
let wallet_birthday = wallet_birthday(conn)?;
// If the chain tip is below the prior max scanned height, then the caller has caught
// the chain in the middle of a reorg. Do nothing; the caller will continue using the
@ -730,11 +741,10 @@ pub(crate) fn update_chain_tip<P: consensus::Parameters>(
// We don't check the shard height, as normal usage would have the caller update the
// shard state prior to this call, so it is possible and expected to be in a situation
// where we should update the tip-related scan ranges but not the shard-related ones.
if let Some(h) = prior_tip {
if new_tip < h {
return Ok(());
}
}
match max_scanned {
Some(h) if new_tip < h => return Ok(()),
_ => (),
};
// `ScanRange` uses an exclusive upper bound.
let chain_end = new_tip + 1;
@ -748,82 +758,115 @@ pub(crate) fn update_chain_tip<P: consensus::Parameters>(
)?;
// Create a scanning range for the fragment of the last shard leading up to new tip.
let shard_entry = shard_start_height
let tip_shard_entry = shard_start_height
.filter(|h| h < &chain_end)
.map(|h| ScanRange::from_parts(h..chain_end, ScanPriority::ChainTip));
// Create scanning ranges to either validate potentially invalid blocks at the wallet's view
// of the chain tip, or connect the prior tip to the new tip.
let tip_entry = prior_tip.map(|prior_tip| {
// If we don't have shard metadata, this means we're doing linear scanning, so create a
// scan range from the prior tip to the current tip with `Historic` priority.
if shard_entry.is_none() {
ScanRange::from_parts(prior_tip..chain_end, ScanPriority::Historic)
} else {
// Determine the height to which we expect blocks retrieved from the block source to be stable
// and not subject to being reorg'ed.
let stable_height = new_tip.saturating_sub(PRUNING_DEPTH);
// Create scan ranges to either validate potentially invalid blocks at the wallet's
// view of the chain tip, or connect the prior tip to the new tip.
let tip_entry = max_scanned.map_or_else(
|| {
// No blocks have been scanned, so we need to anchor the start of the new scan
// range to something else.
wallet_birthday.map_or_else(
// We don't have a wallet birthday, which means we have no accounts yet.
// We can therefore ignore all blocks up to the chain tip.
|| ScanRange::from_parts(sapling_activation..chain_end, ScanPriority::Ignored),
// We have a wallet birthday, so mark all blocks between that and the
// chain tip as `Historic` (performing wallet recovery).
|wallet_birthday| {
ScanRange::from_parts(wallet_birthday..chain_end, ScanPriority::Historic)
},
)
},
|max_scanned| {
// The scan range starts at the block after the max scanned height. Since
// `scan_cached_blocks` retrieves the metadata for the block being connected to
// (if it exists), the connectivity of the scan range to the max scanned block
// will always be checked if relevant.
let min_unscanned = max_scanned + 1;
// If the wallet's prior tip is above the stable height, prioritize the range between
// it and the new tip as `ChainTip`. Otherwise, prioritize the `VERIFY_LOOKAHEAD`
// blocks above the wallet's prior tip as `Verify`. Since `scan_cached_blocks`
// retrieves the metadata for the block being connected to, the connectivity to the
// prior tip will always be checked. Since `Verify` ranges have maximum priority, even
// if the block source begins downloading blocks from the shard scan range (which ends
// at the stable height) the scanner should not attempt to scan those blocks until the
// tip range has been completely checked and any required rewinds have been performed.
if prior_tip >= stable_height {
// This may overlap the `shard_entry` range and if so will be coalesced with it.
ScanRange::from_parts(prior_tip..chain_end, ScanPriority::ChainTip)
// If we don't have shard metadata, this means we're doing linear scanning, so
// create a scan range from the prior tip to the current tip with `Historic`
// priority.
if tip_shard_entry.is_none() {
ScanRange::from_parts(min_unscanned..chain_end, ScanPriority::Historic)
} else {
// The prior tip is in the range that we now expect to be stable, so we need to verify
// and advance it up to at most the stable height. The shard entry will then cover
// the range to the new tip at the lower `ChainTip` priority.
ScanRange::from_parts(
prior_tip..min(stable_height, prior_tip + VERIFY_LOOKAHEAD),
ScanPriority::Verify,
)
// Determine the height to which we expect new blocks retrieved from the
// block source to be stable and not subject to being reorg'ed.
let stable_height = new_tip.saturating_sub(PRUNING_DEPTH);
// If the wallet's max scanned height is above the stable height,
// prioritize the range between it and the new tip as `ChainTip`.
if max_scanned > stable_height {
// We are in the steady-state case, where a wallet is close to the
// chain tip and just needs to catch up.
//
// This overlaps the `tip_shard_entry` range and so will be coalesced
// with it.
ScanRange::from_parts(min_unscanned..chain_end, ScanPriority::ChainTip)
} else {
// In this case, the max scanned height is considered stable relative
// to the chain tip. However, it may be stable or unstable relative to
// the prior chain tip, which we could determine by looking up the
// prior chain tip height from the scan queue. For simplicity we merge
// these two cases together, and proceed as though the max scanned
// block is unstable relative to the prior chain tip.
//
// To confirm its stability, prioritize the `VERIFY_LOOKAHEAD` blocks
// above the max scanned height as `Verify`:
//
// - We use `Verify` to ensure that a connectivity check is performed,
// along with any required rewinds, before any `ChainTip` ranges
// (from this or any prior `update_chain_tip` call) are scanned.
//
// - We prioritize `VERIFY_LOOKAHEAD` blocks because this is expected
// to be 12.5 minutes, within which it is reasonable for a user to
// have potentially received a transaction (if they opened their
// wallet to provide an address to someone else, or spent their own
// funds creating a change output), without necessarily having left
// their wallet open long enough for the transaction to be mined and
// the corresponding block to be scanned.
//
// - We limit the range to at most the stable region, to prevent any
// `Verify` ranges from being susceptible to reorgs, and potentially
// interfering with subsequent `Verify` ranges defined by future
// calls to `update_chain_tip`. Any gap between `stable_height` and
// `shard_start_height` will be filled by the scan range merging
// logic with a `Historic` range.
//
// If `max_scanned == stable_height` then this is a zero-length range.
// In this case, any non-empty `(stable_height+1)..shard_start_height`
// will be marked `Historic`, minimising the prioritised blocks at the
// chain tip and allowing for other ranges (for example, `FoundNote`)
// to take priority.
ScanRange::from_parts(
min_unscanned..min(stable_height + 1, min_unscanned + VERIFY_LOOKAHEAD),
ScanPriority::Verify,
)
}
}
}
});
if let Some(entry) = &shard_entry {
},
);
if let Some(entry) = &tip_shard_entry {
debug!("{} will update latest shard", entry);
}
if let Some(entry) = &tip_entry {
debug!("{} will connect prior tip to new tip", entry);
}
debug!("{} will connect prior scanned state to new tip", tip_entry);
let query_range = match (shard_entry.as_ref(), tip_entry.as_ref()) {
(Some(se), Some(te)) => Some(Range {
start: min(se.block_range().start, te.block_range().start),
end: max(se.block_range().end, te.block_range().end),
}),
(Some(se), None) => Some(se.block_range().clone()),
(None, Some(te)) => Some(te.block_range().clone()),
(None, None) => None,
let query_range = match tip_shard_entry.as_ref() {
Some(se) => Range {
start: min(se.block_range().start, tip_entry.block_range().start),
end: max(se.block_range().end, tip_entry.block_range().end),
},
None => tip_entry.block_range().clone(),
};
if let Some(query_range) = query_range {
replace_queue_entries::<SqliteClientError>(
conn,
&query_range,
shard_entry.into_iter().chain(tip_entry.into_iter()),
false,
)?;
} else {
// If we have neither shard data nor any existing block data in the database, we should also
// have no existing scan queue entries and can fall back to linear scanning from Sapling
// activation.
if let Some(sapling_activation) = params.activation_height(NetworkUpgrade::Sapling) {
// If the caller provided a chain tip that is before Sapling activation, do
// nothing.
if sapling_activation < chain_end {
let scan_range =
ScanRange::from_parts(sapling_activation..chain_end, ScanPriority::Historic);
insert_queue_entries(conn, Some(scan_range).iter())?;
}
}
}
replace_queue_entries::<SqliteClientError>(
conn,
&query_range,
tip_shard_entry.into_iter().chain(Some(tip_entry)),
false,
)?;
Ok(())
}
@ -834,6 +877,7 @@ mod tests {
use incrementalmerkletree::{frontier::Frontier, Hashable, Level, Position};
use secrecy::SecretVec;
use zcash_client_backend::data_api::{
chain::CommitmentTreeRoot,
scanning::{ScanPriority, ScanRange},
@ -844,12 +888,14 @@ mod tests {
consensus::{BlockHeight, NetworkUpgrade, Parameters},
sapling::Node,
transaction::components::Amount,
zip32::DiversifiableFullViewingKey,
};
use crate::{
error::SqliteClientError,
testing::{AddressType, TestBuilder},
testing::{AddressType, BlockCache, TestBuilder, TestState},
wallet::scanning::{insert_queue_entries, replace_queue_entries, suggest_scan_ranges},
VERIFY_LOOKAHEAD,
};
use super::{join_nonoverlapping, Joined, RangeOrdering, SpanningTree};
@ -1047,6 +1093,30 @@ mod tests {
);
}
#[test]
fn spanning_tree_insert_gaps() {
use ScanPriority::*;
let t = spanning_tree(&[(0..3, Historic), (6..8, ChainTip)]).unwrap();
assert_eq!(
t.into_vec(),
vec![scan_range(0..6, Historic), scan_range(6..8, ChainTip),]
);
let t = spanning_tree(&[(0..3, Historic), (3..4, Verify), (6..8, ChainTip)]).unwrap();
assert_eq!(
t.into_vec(),
vec![
scan_range(0..3, Historic),
scan_range(3..4, Verify),
scan_range(4..6, Historic),
scan_range(6..8, ChainTip),
]
);
}
#[test]
fn spanning_tree_insert_rfd_span() {
use ScanPriority::*;
@ -1314,18 +1384,20 @@ mod tests {
assert_matches!(
st.wallet().suggest_scan_ranges(),
Ok(scan_ranges) if scan_ranges == vec![
scan_range((sap_active + 319)..(sap_active + 329), Verify),
scan_range((sap_active + 329)..(sap_active + 451), ChainTip),
scan_range((sap_active + 320)..(sap_active + 330), Verify),
scan_range((sap_active + 330)..(sap_active + 451), ChainTip),
scan_range((sap_active + 300)..(sap_active + 310), ChainTip)
]
);
}
#[test]
fn create_account_creates_ignored_range() {
use ScanPriority::*;
let mut st = TestBuilder::new()
fn test_with_canopy_birthday() -> (
TestState<BlockCache>,
DiversifiableFullViewingKey,
AccountBirthday,
u32,
) {
let st = TestBuilder::new()
.with_block_cache()
.with_test_account(|network| {
// We use Canopy activation as an arbitrary birthday height that's greater than Sapling
@ -1346,27 +1418,190 @@ mod tests {
let dfvk = st.test_account_sapling().unwrap();
let sap_active = st.sapling_activation_height();
(st, dfvk, birthday, sap_active.into())
}
#[test]
fn create_account_creates_ignored_range() {
use ScanPriority::*;
let (st, _, birthday, sap_active) = test_with_canopy_birthday();
let birthday_height = birthday.height().into();
let expected = vec![
// The range up to the wallet's birthday height is ignored.
scan_range(u32::from(sap_active)..u32::from(birthday.height()), Ignored),
scan_range(sap_active..birthday_height, Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
}
#[test]
fn update_chain_tip_before_create_account() {
use ScanPriority::*;
let mut st = TestBuilder::new().with_block_cache().build();
let sap_active = st.sapling_activation_height();
// Update the chain tip.
let new_tip = sap_active + 1000;
st.wallet_mut().update_chain_tip(new_tip).unwrap();
let chain_end = u32::from(new_tip + 1);
let expected = vec![
// The range up to the chain end is ignored.
scan_range(sap_active.into()..chain_end, Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
// Set up some shard root history before the wallet birthday
// Now add an account.
let wallet_birthday = sap_active + 500;
st.wallet_mut()
.create_account(
&SecretVec::new(vec![0; 32]),
AccountBirthday::from_parts(wallet_birthday, Frontier::empty(), None),
)
.unwrap();
let expected = vec![
// The account's birthday onward is marked for recovery.
scan_range(wallet_birthday.into()..chain_end, Historic),
// The range up to the wallet's birthday height is ignored.
scan_range(sap_active.into()..wallet_birthday.into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
}
#[test]
fn update_chain_tip_with_no_subtree_roots() {
use ScanPriority::*;
let (mut st, _, birthday, sap_active) = test_with_canopy_birthday();
// Set up the following situation:
//
// prior_tip new_tip
// |<--- 500 --->|
// wallet_birthday
let prior_tip = birthday.height();
let wallet_birthday = birthday.height().into();
// Update the chain tip.
let new_tip = prior_tip + 500;
st.wallet_mut().update_chain_tip(new_tip).unwrap();
let chain_end = u32::from(new_tip + 1);
// Verify that the suggested scan ranges match what is expected.
let expected = vec![
// The wallet's birthday onward is marked for recovery.
scan_range(wallet_birthday..chain_end, Historic),
// The range below the wallet's birthday height is ignored.
scan_range(sap_active..wallet_birthday, Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
}
#[test]
fn update_chain_tip_when_never_scanned() {
use ScanPriority::*;
let (mut st, _, birthday, sap_active) = test_with_canopy_birthday();
// Set up the following situation:
//
// last_shard_start prior_tip new_tip
// |<----- 1000 ----->|<--- 500 --->|
// wallet_birthday
let prior_tip_height = birthday.height();
// Set up some shard root history before the wallet birthday.
let last_shard_start = birthday.height() - 1000;
st.wallet_mut()
.put_sapling_subtree_roots(
0,
&[CommitmentTreeRoot::from_parts(
birthday.height() - 1000,
last_shard_start,
// fake a hash, the value doesn't matter
Node::empty_leaf(),
)],
)
.unwrap();
// Update the chain tip.
let tip_height = prior_tip_height + 500;
st.wallet_mut().update_chain_tip(tip_height).unwrap();
let chain_end = u32::from(tip_height + 1);
// Verify that the suggested scan ranges match what is expected.
let expected = vec![
// The entire last (incomplete) shard's range is marked for catching up to the
// chain tip, to ensure that if any notes are discovered after the wallet's
// birthday, they will be spendable.
scan_range(last_shard_start.into()..chain_end, ChainTip),
// The range below the last shard is ignored.
scan_range(sap_active..last_shard_start.into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
}
#[test]
fn update_chain_tip_unstable_max_scanned() {
use ScanPriority::*;
let (mut st, dfvk, birthday, sap_active) = test_with_canopy_birthday();
// Set up the following situation:
//
// prior_tip new_tip
// |<--- 500 --->|<- 40 ->|<-- 70 -->|<- 20 ->|
// wallet_birthday max_scanned last_shard_start
//
let max_scanned = birthday.height() + 500;
let prior_tip = max_scanned + 40;
// Set up some shard root history before the wallet birthday.
let second_to_last_shard_start = birthday.height() - 1000;
st.wallet_mut()
.put_sapling_subtree_roots(
0,
&[CommitmentTreeRoot::from_parts(
second_to_last_shard_start,
// fake a hash, the value doesn't matter
Node::empty_leaf(),
)],
)
.unwrap();
// Set up prior chain state. This simulates us having imported a wallet
// with a birthday 520 blocks below the chain tip.
st.wallet_mut().update_chain_tip(prior_tip).unwrap();
// Verify that the suggested scan ranges match what is expected.
let expected = vec![
// The second-to-last shard is currently the last shard, so it is marked for
// scanning to catch up to the prior chain tip. This includes heights prior to
// the wallet's birthday, because the wallet doesn't know that it already has
// enough data from the initial frontier to avoid having to scan this range.
scan_range(
second_to_last_shard_start.into()..(prior_tip + 1).into(),
ChainTip,
),
// The range below the second-to-last shard is ignored.
scan_range(sap_active..second_to_last_shard_start.into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
// Now, scan the max scanned block.
st.generate_block_at(
birthday.height(),
max_scanned,
BlockHash([0u8; 32]),
&dfvk,
AddressType::DefaultExternal,
@ -1375,42 +1610,150 @@ mod tests {
.try_into()
.unwrap(),
);
st.scan_cached_blocks(birthday.height(), 1);
st.scan_cached_blocks(max_scanned, 1);
// Update the chain tip
let tip_height = st
.wallet()
.params
.activation_height(NetworkUpgrade::Nu5)
// Now simulate shutting down, and then restarting 90 blocks later, after a shard
// has been completed.
let last_shard_start = prior_tip + 70;
st.wallet_mut()
.put_sapling_subtree_roots(
0,
&[CommitmentTreeRoot::from_parts(
last_shard_start,
// fake a hash, the value doesn't matter
Node::empty_leaf(),
)],
)
.unwrap();
st.wallet_mut().update_chain_tip(tip_height).unwrap();
let new_tip = last_shard_start + 20;
st.wallet_mut().update_chain_tip(new_tip).unwrap();
let chain_end = u32::from(new_tip + 1);
// Verify that the suggested scan ranges match what is expected
let expected = vec![
// The birthday height is the "first to be scanned" (as the wallet birthday),
// so we verify 10 blocks starting at that height.
// The max scanned block's connectivity is verified by scanning the next 10 blocks.
scan_range(
u32::from(birthday.height())..u32::from(birthday.height() + 10),
(max_scanned + 1).into()..(max_scanned + 1 + VERIFY_LOOKAHEAD).into(),
Verify,
),
// The remainder of the shard after the verify segment is required in order to make
// notes spendable, so it has priority `ChainTip`
// The last shard needs to catch up to the chain tip in order to make notes spendable.
scan_range(last_shard_start.into()..chain_end, ChainTip),
// The range between the verification blocks and the prior tip is still in the queue.
scan_range(
u32::from(birthday.height() + 10)..u32::from(tip_height + 1),
(max_scanned + 1 + VERIFY_LOOKAHEAD).into()..(prior_tip + 1).into(),
ChainTip,
),
// The remainder of the shard prior to the birthday height must be scanned because the
// wallet doesn't know that it already has enough data from the initial frontier to
// avoid having to scan this range.
// The remainder of the second-to-last shard's range is still in the queue.
scan_range(
u32::from(birthday.height() - 1000)..u32::from(birthday.height()),
second_to_last_shard_start.into()..max_scanned.into(),
ChainTip,
),
// The range below the wallet's birthday height is ignored
// The gap between the prior tip and the last shard is deferred as low priority.
scan_range((prior_tip + 1).into()..last_shard_start.into(), Historic),
// The max scanned block itself is left as-is.
scan_range(max_scanned.into()..(max_scanned + 1).into(), Scanned),
// The range below the second-to-last shard is ignored.
scan_range(sap_active..second_to_last_shard_start.into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
}
#[test]
fn update_chain_tip_stable_max_scanned() {
use ScanPriority::*;
let (mut st, dfvk, birthday, sap_active) = test_with_canopy_birthday();
// Set up the following situation:
//
// prior_tip new_tip
// |<--- 500 --->|<- 20 ->|<-- 50 -->|<- 20 ->|
// wallet_birthday max_scanned last_shard_start
//
let max_scanned = birthday.height() + 500;
let prior_tip = max_scanned + 20;
// Set up some shard root history before the wallet birthday.
let second_to_last_shard_start = birthday.height() - 1000;
st.wallet_mut()
.put_sapling_subtree_roots(
0,
&[CommitmentTreeRoot::from_parts(
second_to_last_shard_start,
// fake a hash, the value doesn't matter
Node::empty_leaf(),
)],
)
.unwrap();
// Set up prior chain state. This simulates us having imported a wallet
// with a birthday 520 blocks below the chain tip.
st.wallet_mut().update_chain_tip(prior_tip).unwrap();
// Verify that the suggested scan ranges match what is expected.
let expected = vec![
// The second-to-last shard is currently the last shard, so it is marked for
// scanning to catch up to the prior chain tip. This includes heights prior to
// the wallet's birthday, because the wallet doesn't know that it already has
// enough data from the initial frontier to avoid having to scan this range.
scan_range(
u32::from(sap_active)..u32::from(birthday.height() - 1000),
Ignored,
second_to_last_shard_start.into()..(prior_tip + 1).into(),
ChainTip,
),
// The range below the second-to-last shard is ignored.
scan_range(sap_active..second_to_last_shard_start.into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();
assert_eq!(actual, expected);
// Now, scan the max scanned block.
st.generate_block_at(
max_scanned,
BlockHash([0u8; 32]),
&dfvk,
AddressType::DefaultExternal,
Amount::const_from_i64(10000),
u64::from(birthday.sapling_frontier().value().unwrap().position() + 1)
.try_into()
.unwrap(),
);
st.scan_cached_blocks(max_scanned, 1);
// Now simulate shutting down, and then restarting 70 blocks later, after a shard
// has been completed.
let last_shard_start = prior_tip + 50;
st.wallet_mut()
.put_sapling_subtree_roots(
0,
&[CommitmentTreeRoot::from_parts(
last_shard_start,
// fake a hash, the value doesn't matter
Node::empty_leaf(),
)],
)
.unwrap();
let new_tip = last_shard_start + 20;
st.wallet_mut().update_chain_tip(new_tip).unwrap();
let chain_end = u32::from(new_tip + 1);
// Verify that the suggested scan ranges match what is expected.
let expected = vec![
// The blocks after the max scanned block up to the chain tip are prioritised.
scan_range((max_scanned + 1).into()..chain_end, ChainTip),
// The remainder of the second-to-last shard's range is still in the queue.
scan_range(
second_to_last_shard_start.into()..max_scanned.into(),
ChainTip,
),
// The max scanned block itself is left as-is.
scan_range(max_scanned.into()..(max_scanned + 1).into(), Scanned),
// The range below the second-to-last shard is ignored.
scan_range(sap_active..second_to_last_shard_start.into(), Ignored),
];
let actual = suggest_scan_ranges(&st.wallet().conn, Ignored).unwrap();