Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve set_scripts delete bug #193

Merged
merged 2 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,6 @@ impl Storage {

pub fn update_filter_scripts(&self, scripts: Vec<ScriptStatus>, command: SetScriptsCommand) {
let mut should_filter_genesis_block = false;
let mut min_block_number = None;
let mut batch = self.batch();
let key_prefix = Key::Meta(FILTER_SCRIPTS_KEY).into_vec();

Expand All @@ -288,13 +287,6 @@ impl Storage {
});

for ss in scripts {
if min_block_number
.as_ref()
.map(|n| *n > ss.block_number)
.unwrap_or(true)
{
min_block_number = Some(ss.block_number);
}
let key = [
key_prefix.as_ref(),
ss.script.as_slice(),
Expand All @@ -315,12 +307,6 @@ impl Storage {
}
let min_script_block_number = scripts.iter().map(|ss| ss.block_number).min();
should_filter_genesis_block = min_script_block_number == Some(0);
// min_block_number should be the min of all scripts' block_number when storage's filter_scripts is empty
min_block_number = if self.is_filter_scripts_empty() {
min_script_block_number
} else {
min_script_block_number.map(|n| n.min(self.get_min_filtered_block_number()))
};

for ss in scripts {
let key = [
Expand All @@ -341,6 +327,7 @@ impl Storage {
if scripts.is_empty() {
return;
}

for ss in scripts {
let key = [
key_prefix.as_ref(),
Expand All @@ -358,9 +345,7 @@ impl Storage {

batch.commit().expect("batch commit should be ok");

if let Some(min_number) = min_block_number {
self.update_min_filtered_block_number(min_number);
}
self.update_min_filtered_block_number_by_scripts();
self.clear_matched_blocks();

if should_filter_genesis_block {
Expand All @@ -369,6 +354,24 @@ impl Storage {
}
}

fn update_min_filtered_block_number_by_scripts(&self) {
let key_prefix = Key::Meta(FILTER_SCRIPTS_KEY).into_vec();
let mode = IteratorMode::From(key_prefix.as_ref(), Direction::Forward);

let min_block_number = self
.db
.iterator(mode)
.take_while(|(key, _value)| key.starts_with(&key_prefix))
.map(|(_key, value)| {
BlockNumber::from_be_bytes(value.as_ref().try_into().expect("stored BlockNumber"))
})
.min();

if let Some(n) = min_block_number {
self.update_min_filtered_block_number(n);
}
}

// get scripts hash that should be filtered below the given block number
pub fn get_scripts_hash(&self, block_number: BlockNumber) -> Vec<Byte32> {
let key_prefix = Key::Meta(FILTER_SCRIPTS_KEY).into_vec();
Expand Down
47 changes: 47 additions & 0 deletions src/tests/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,53 @@ fn test_set_scripts_partial_min_filtered_block_number_bug() {
assert_eq!(storage.get_min_filtered_block_number(), 1234,);
}

#[test]
fn test_set_scripts_delete_min_filtered_block_number_bug() {
let storage = new_storage("set_scripts_delete_min_filtered_block_number_bug");
let peers = create_peers();
let swc = StorageWithChainData::new(storage.clone(), Arc::clone(&peers), Default::default());
let rpc = BlockFilterRpcImpl { swc };

storage.update_min_filtered_block_number(42);
rpc.set_scripts(
vec![
ScriptStatus {
script: Script::new_builder()
.args(Bytes::from("abc").pack())
.build()
.into(),
script_type: ScriptType::Lock,
block_number: 1234.into(),
},
ScriptStatus {
script: Script::new_builder()
.args(Bytes::from("xyz").pack())
.build()
.into(),
script_type: ScriptType::Type,
block_number: 5678.into(),
},
],
Some(SetScriptsCommand::All),
)
.unwrap();
assert_eq!(storage.get_min_filtered_block_number(), 1234,);

rpc.set_scripts(
vec![ScriptStatus {
script: Script::new_builder()
.args(Bytes::from("abc").pack())
.build()
.into(),
script_type: ScriptType::Lock,
block_number: 1234.into(),
}],
Some(SetScriptsCommand::Delete),
)
.unwrap();
assert_eq!(storage.get_min_filtered_block_number(), 5678,);
}

#[test]
fn test_chain_txs_in_same_block_bug() {
let storage = new_storage("chain_txs_in_same_block_bug");
Expand Down
Loading