Skip to content

Commit

Permalink
fix: correctly calculate evaluations for predicate status (#424)
Browse files Browse the repository at this point in the history
### Description

Previously, when the unconfirmed predicate status was set after the
status originally was scanning, we were adding the number of evaluated
blocks with the previous statuses number of blocks. When scanning, we
have a running total of blocks scanned, so we don't need to use the
previously stored data.

---

### Checklist

- [x] All tests pass
- [x] Tests added in this PR (if applicable)
  • Loading branch information
MicaiahReid authored Oct 4, 2023
1 parent 1e606ed commit ec76f29
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 54 deletions.
18 changes: 9 additions & 9 deletions components/chainhook-cli/src/scan/bitcoin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,15 @@ pub async fn scan_bitcoin_chainstate_via_rpc_using_predicate(
);

if let Some(ref mut predicates_db_conn) = predicates_db_conn {
set_predicate_scanning_status(
&predicate_spec.key(),
number_of_blocks_to_scan,
number_of_blocks_scanned,
number_of_times_triggered,
last_block_scanned.index,
predicates_db_conn,
ctx,
);
if let Some(predicate_end_block) = predicate_spec.end_block {
if predicate_end_block == last_block_scanned.index {
// todo: we need to find a way to check if this block is confirmed
Expand All @@ -237,15 +246,6 @@ pub async fn scan_bitcoin_chainstate_via_rpc_using_predicate(
return Ok(true);
}
}
set_predicate_scanning_status(
&predicate_spec.key(),
number_of_blocks_to_scan,
number_of_blocks_scanned,
number_of_times_triggered,
last_block_scanned.index,
predicates_db_conn,
ctx,
);
}

return Ok(false);
Expand Down
18 changes: 9 additions & 9 deletions components/chainhook-cli/src/scan/stacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,15 @@ pub async fn scan_stacks_chainstate_via_rocksdb_using_predicate(
);

if let Some(ref mut predicates_db_conn) = predicates_db_conn {
set_predicate_scanning_status(
&predicate_spec.key(),
number_of_blocks_to_scan,
number_of_blocks_scanned,
number_of_times_triggered,
last_block_scanned.index,
predicates_db_conn,
ctx,
);
if let Some(predicate_end_block) = predicate_spec.end_block {
if predicate_end_block == last_block_scanned.index {
let is_confirmed = match get_stacks_block_at_block_height(
Expand Down Expand Up @@ -412,15 +421,6 @@ pub async fn scan_stacks_chainstate_via_rocksdb_using_predicate(
return Ok((last_block_scanned, true));
}
}
set_predicate_scanning_status(
&predicate_spec.key(),
number_of_blocks_to_scan,
number_of_blocks_scanned,
number_of_times_triggered,
last_block_scanned.index,
predicates_db_conn,
ctx,
);
}
Ok((last_block_scanned, false))
}
Expand Down
9 changes: 3 additions & 6 deletions components/chainhook-cli/src/service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,9 +838,7 @@ pub fn set_predicate_scanning_status(
);
}

/// Updates a predicate's status to `InitialScanCompleted`.
///
/// Preserves the scanning metrics from the predicate's previous status
/// Updates a predicate's status to `UnconfirmedExpiration`.
pub fn set_unconfirmed_expiration_status(
chain: &Chain,
number_of_new_blocks_evaluated: u64,
Expand All @@ -860,12 +858,12 @@ pub fn set_unconfirmed_expiration_status(
Some(status) => match status {
PredicateStatus::Scanning(ScanningData {
number_of_blocks_to_scan: _,
number_of_blocks_evaluated,
number_of_blocks_evaluated: _,
number_of_times_triggered,
last_occurrence,
last_evaluated_block_height,
}) => (
number_of_blocks_evaluated + number_of_new_blocks_evaluated,
number_of_new_blocks_evaluated,
number_of_times_triggered,
last_occurrence,
last_evaluated_block_height,
Expand Down Expand Up @@ -1042,7 +1040,6 @@ pub fn update_predicate_status(
ctx: &Context,
) {
let serialized_status = json!(status).to_string();
println!("serialized predicate status {serialized_status}");
if let Err(e) =
predicates_db_conn.hset::<_, _, _, ()>(&predicate_key, "status", &serialized_status)
{
Expand Down
121 changes: 91 additions & 30 deletions components/chainhook-cli/src/service/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,27 +261,84 @@ async fn it_handles_stacks_predicates_with_filters(filters: JsonValue) {
}
}

fn assert_confirmed_expiration_status(status: PredicateStatus) {
fn assert_confirmed_expiration_status(
(status, expected_evaluations, expected_occurrences): (
PredicateStatus,
Option<u64>,
Option<u64>,
),
) {
match status {
PredicateStatus::ConfirmedExpiration(_) => {}
PredicateStatus::ConfirmedExpiration(data) => {
if let Some(expected) = expected_evaluations {
assert_eq!(
data.number_of_blocks_evaluated, expected,
"incorrect number of blocks evaluated"
);
}
if let Some(expected) = expected_occurrences {
assert_eq!(
data.number_of_times_triggered, expected,
"incorrect number of predicates triggered"
);
}
}
_ => panic!("expected ConfirmedExpiration status, found {:?}", status),
}
}
fn assert_unconfirmed_expiration_status(status: PredicateStatus) {
fn assert_unconfirmed_expiration_status(
(status, expected_evaluations, expected_occurrences): (
PredicateStatus,
Option<u64>,
Option<u64>,
),
) {
match status {
PredicateStatus::UnconfirmedExpiration(_) => {}
PredicateStatus::UnconfirmedExpiration(data) => {
if let Some(expected) = expected_evaluations {
assert_eq!(
data.number_of_blocks_evaluated, expected,
"incorrect number of blocks evaluated"
);
}
if let Some(expected) = expected_occurrences {
assert_eq!(
data.number_of_times_triggered, expected,
"incorrect number of predicates triggered"
);
}
}
_ => panic!("expected UnconfirmedExpiration status, found {:?}", status),
}
}

fn assert_streaming_status(status: PredicateStatus) {
fn assert_streaming_status(
(status, expected_evaluations, expected_occurrences): (
PredicateStatus,
Option<u64>,
Option<u64>,
),
) {
match status {
PredicateStatus::Streaming(_) => {}
PredicateStatus::Streaming(data) => {
if let Some(expected) = expected_evaluations {
assert_eq!(
data.number_of_blocks_evaluated, expected,
"incorrect number of blocks evaluated"
);
}
if let Some(expected) = expected_occurrences {
assert_eq!(
data.number_of_times_triggered, expected,
"incorrect number of predicates triggered"
);
}
}
_ => panic!("expected Streaming status, found {:?}", status),
}
}

fn assert_interrupted_status(status: PredicateStatus) {
fn assert_interrupted_status((status, _, _): (PredicateStatus, Option<u64>, Option<u64>)) {
match status {
PredicateStatus::Interrupted(_) => {}
_ => panic!("expected Interrupted status, found {:?}", status),
Expand Down Expand Up @@ -422,21 +479,23 @@ async fn setup_stacks_chainhook_test(
)
}

#[test_case(5, 0, Some(1), Some(3) => using assert_confirmed_expiration_status; "predicate_end_block lower than starting_chain_tip ends with ConfirmedExpiration status")]
#[test_case(5, 0, Some(1), None => using assert_streaming_status; "no predicate_end_block ends with Streaming status")]
#[test_case(3, 0, Some(1), Some(5) => using assert_streaming_status; "predicate_end_block greater than chain_tip ends with Streaming status")]
#[test_case(5, 3, Some(1), Some(7) => using assert_unconfirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining until end_block ends with UnconfirmedExpiration status")]
#[test_case(1, 3, Some(1), Some(3) => using assert_unconfirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations < CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with UnconfirmedExpiration status")]
#[test_case(3, 7, Some(1), Some(4) => using assert_confirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations >= CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with ConfirmedExpiration status")]
#[test_case(0, 0, None, None => using assert_interrupted_status; "ommitting start_block ends with Interrupted status")]
#[test_case(5, 0, Some(1), Some(3), Some(3), Some(3) => using assert_confirmed_expiration_status; "predicate_end_block lower than starting_chain_tip ends with ConfirmedExpiration status")]
#[test_case(5, 0, Some(1), None, Some(5), Some(5) => using assert_streaming_status; "no predicate_end_block ends with Streaming status")]
#[test_case(3, 0, Some(1), Some(5), Some(3), Some(3) => using assert_streaming_status; "predicate_end_block greater than chain_tip ends with Streaming status")]
#[test_case(5, 4, Some(1), Some(7), Some(9), Some(7) => using assert_unconfirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining until end_block ends with UnconfirmedExpiration status")]
#[test_case(1, 3, Some(1), Some(3), Some(4), Some(3) => using assert_unconfirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations < CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with UnconfirmedExpiration status")]
#[test_case(3, 7, Some(1), Some(4), Some(9), Some(4) => using assert_confirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations >= CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with ConfirmedExpiration status")]
#[test_case(0, 0, None, None, None, None => using assert_interrupted_status; "ommitting start_block ends with Interrupted status")]
#[tokio::test]
#[cfg_attr(not(feature = "redis_tests"), ignore)]
async fn test_stacks_predicate_status_is_updated(
starting_chain_tip: u64,
blocks_to_mine: u64,
predicate_start_block: Option<u64>,
predicate_end_block: Option<u64>,
) -> PredicateStatus {
expected_evaluations: Option<u64>,
expected_occurrences: Option<u64>,
) -> (PredicateStatus, Option<u64>, Option<u64>) {
let (
mut redis_process,
working_dir,
Expand All @@ -449,7 +508,7 @@ async fn test_stacks_predicate_status_is_updated(
let uuid = &get_random_uuid();
let predicate = build_stacks_payload(
Some("devnet"),
Some(json!({"scope":"block_height", "lower_than": 100})),
Some(json!({"scope":"block_height", "lower_than": 600})),
None,
Some(json!({"start_block": predicate_start_block, "end_block": predicate_end_block})),
Some(uuid),
Expand Down Expand Up @@ -510,7 +569,7 @@ async fn test_stacks_predicate_status_is_updated(
std::fs::remove_dir_all(&working_dir).unwrap();
flush_redis(redis_port);
redis_process.kill().unwrap();
result
(result, expected_evaluations, expected_occurrences)
}

async fn setup_bitcoin_chainhook_test(
Expand Down Expand Up @@ -577,19 +636,21 @@ async fn setup_bitcoin_chainhook_test(
)
}

#[test_case(5, 1, Some(1), Some(3) => using assert_unconfirmed_expiration_status; "predicate_end_block lower than starting_chain_tip with predicate_end_block confirmations < CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with UnconfirmedExpiration status")]
#[test_case(10, 1, Some(1), Some(3) => using assert_confirmed_expiration_status; "predicate_end_block lower than starting_chain_tip with predicate_end_block confirmations >= CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with ConfirmedExpiration status")]
#[test_case(1, 3, Some(1), Some(3) => using assert_unconfirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations < CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with UnconfirmedExpiration status")]
#[test_case(3, 7, Some(1), Some(4) => using assert_confirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations >= CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with ConfirmedExpiration status")]
#[test_case(0, 0, None, None => using assert_interrupted_status; "ommitting start_block ends with Interrupted status")]
#[test_case(5, 1, Some(1), Some(3), Some(3), Some(3) => using assert_unconfirmed_expiration_status; "predicate_end_block lower than starting_chain_tip with predicate_end_block confirmations < CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with UnconfirmedExpiration status")]
#[test_case(10, 1, Some(1), Some(3), Some(3), Some(3) => using assert_confirmed_expiration_status; "predicate_end_block lower than starting_chain_tip with predicate_end_block confirmations >= CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with ConfirmedExpiration status")]
#[test_case(1, 3, Some(1), Some(3), Some(4), Some(3) => using assert_unconfirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations < CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with UnconfirmedExpiration status")]
#[test_case(3, 7, Some(1), Some(4), Some(9), Some(4) => using assert_confirmed_expiration_status; "predicate_end_block greater than starting_chain_tip and mining blocks so that predicate_end_block confirmations >= CONFIRMED_SEGMENT_MINIMUM_LENGTH ends with ConfirmedExpiration status")]
#[test_case(0, 0, None, None, None, None => using assert_interrupted_status; "ommitting start_block ends with Interrupted status")]
#[tokio::test]
#[cfg_attr(not(feature = "redis_tests"), ignore)]
async fn test_bitcoin_predicate_status_is_updated(
starting_chain_tip: u64,
blocks_to_mine: u64,
predicate_start_block: Option<u64>,
predicate_end_block: Option<u64>,
) -> PredicateStatus {
expected_evaluations: Option<u64>,
expected_occurrences: Option<u64>,
) -> (PredicateStatus, Option<u64>, Option<u64>) {
let (
mut redis_process,
working_dir,
Expand Down Expand Up @@ -666,7 +727,7 @@ async fn test_bitcoin_predicate_status_is_updated(
std::fs::remove_dir_all(&working_dir).unwrap();
flush_redis(redis_port);
redis_process.kill().unwrap();
result
(result, expected_evaluations, expected_occurrences)
}

///
Expand Down Expand Up @@ -745,7 +806,7 @@ async fn test_bitcoin_predicate_status_is_updated_with_reorg(
redis_process.kill().unwrap();
panic!("test failed with error: {e}");
});
assert_streaming_status(status);
assert_streaming_status((status, None, None));

let branch_key = '1';
let first_fork_block_mined_height = fork_point + 1;
Expand Down Expand Up @@ -793,7 +854,7 @@ async fn test_bitcoin_predicate_status_is_updated_with_reorg(
redis_process.kill().unwrap();
panic!("test failed with error: {e}");
});
assert_streaming_status(status);
assert_streaming_status((status, None, None));
}
}

Expand All @@ -807,7 +868,7 @@ async fn test_bitcoin_predicate_status_is_updated_with_reorg(
panic!("test failed with error: {e}");
});

assert_confirmed_expiration_status(status);
assert_confirmed_expiration_status((status, None, None));

std::fs::remove_dir_all(&working_dir).unwrap();
flush_redis(redis_port);
Expand Down Expand Up @@ -927,7 +988,7 @@ async fn test_deregister_predicate(chain: Chain) {
async fn test_restarting_with_saved_predicates(
starting_status: PredicateStatus,
starting_chain_tip: u64,
) -> PredicateStatus {
) -> (PredicateStatus, Option<u64>, Option<u64>) {
let uuid = &get_random_uuid();
let predicate = build_stacks_payload(
Some("devnet"),
Expand Down Expand Up @@ -965,7 +1026,7 @@ async fn test_restarting_with_saved_predicates(
std::fs::remove_dir_all(&working_dir).unwrap();
flush_redis(redis_port);
redis_process.kill().unwrap();
result
(result, None, None)
}

#[tokio::test]
Expand Down Expand Up @@ -1007,7 +1068,7 @@ async fn it_allows_specifying_startup_predicate() {
std::fs::remove_dir_all(&working_dir).unwrap();
flush_redis(redis_port);
redis_process.kill().unwrap();
assert_confirmed_expiration_status(result);
assert_confirmed_expiration_status((result, None, None));
}

#[tokio::test]
Expand Down

0 comments on commit ec76f29

Please sign in to comment.