From 947b11ee989b92fcaab8a4d1d39e83c62a674bd7 Mon Sep 17 00:00:00 2001 From: Micaiah Reid Date: Mon, 9 Oct 2023 13:37:57 -0400 Subject: [PATCH] fix: make `include_contract_abi` optional (#432) ### Description The new field `include_contract_abi` already was optional for the user-facing type `StacksChainhookNetworkSpecification`, but was not optional for the internal type `StacksChainhookSpecification`. Any time we converted to a `StacksChainhookSpecification`, we'd `.unwrap_or(false)`, which seemed okay. However, a running Chainhook node has the `StacksChainhookSpecification` type saved in redis, so whenever a pre-existing node is restarted after upgrading, it is missing the required `include_contract_abi` field and errors. This PR makes the field optional everywhere. I'll need to think about how to add tests to prevent this sort of bug in the future. ### Checklist - [x] All tests pass - [ ] Tests added in this PR (if applicable) --- .../chainhook-sdk/src/chainhooks/stacks/mod.rs | 2 +- .../chainhook-sdk/src/chainhooks/tests/mod.rs | 16 ++++++++-------- components/chainhook-sdk/src/chainhooks/types.rs | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/components/chainhook-sdk/src/chainhooks/stacks/mod.rs b/components/chainhook-sdk/src/chainhooks/stacks/mod.rs index 6f0cd47c5..345c80369 100644 --- a/components/chainhook-sdk/src/chainhooks/stacks/mod.rs +++ b/components/chainhook-sdk/src/chainhooks/stacks/mod.rs @@ -795,7 +795,7 @@ pub fn serialize_stacks_payload_to_json<'a>( ctx: &Context, ) -> JsonValue { let decode_clarity_values = trigger.should_decode_clarity_value(); - let include_contract_abi = trigger.chainhook.include_contract_abi; + let include_contract_abi = trigger.chainhook.include_contract_abi.unwrap_or(false); json!({ "apply": trigger.apply.into_iter().map(|(transactions, block)| { serialize_stacks_block(block, transactions, decode_clarity_values, include_contract_abi, ctx) diff --git a/components/chainhook-sdk/src/chainhooks/tests/mod.rs b/components/chainhook-sdk/src/chainhooks/tests/mod.rs index 6fb06e65a..cf564b067 100644 --- a/components/chainhook-sdk/src/chainhooks/tests/mod.rs +++ b/components/chainhook-sdk/src/chainhooks/tests/mod.rs @@ -348,7 +348,7 @@ fn test_stacks_predicates( expire_after_occurrence: None, capture_all_events: None, decode_clarity_values: None, - include_contract_abi: false, + include_contract_abi: None, predicate: predicate, action: HookAction::Noop, enabled: true, @@ -428,7 +428,7 @@ fn test_stacks_predicate_contract_deploy(predicate: StacksPredicate, expected_ap expire_after_occurrence: None, capture_all_events: None, decode_clarity_values: None, - include_contract_abi: false, + include_contract_abi: None, predicate: predicate, action: HookAction::Noop, enabled: true, @@ -483,7 +483,7 @@ fn verify_optional_addition_of_contract_abi() { expire_after_occurrence: None, capture_all_events: None, decode_clarity_values: None, - include_contract_abi: true, + include_contract_abi: Some(true), predicate: StacksPredicate::ContractDeployment( StacksContractDeploymentPredicate::Deployer("*".to_string()), ), @@ -503,7 +503,7 @@ fn verify_optional_addition_of_contract_abi() { expire_after_occurrence: None, capture_all_events: None, decode_clarity_values: None, - include_contract_abi: true, + include_contract_abi: Some(true), predicate: StacksPredicate::ContractCall(StacksContractCallBasedPredicate { contract_identifier: "ST13F481SBR0R7Z6NMMH8YV2FJJYXA5JPA0AD3HP9.subnet-v1".to_string(), method: "commit-block".to_string(), @@ -537,7 +537,7 @@ fn verify_optional_addition_of_contract_abi() { } } } - contract_deploy_chainhook.include_contract_abi = false; + contract_deploy_chainhook.include_contract_abi = Some(false); let predicates = vec![&contract_deploy_chainhook, &contract_call_chainhook]; let (triggered, _blocks, _) = evaluate_stacks_chainhooks_on_chain_event(&event, predicates, &Context::empty()); @@ -622,7 +622,7 @@ fn test_stacks_predicate_contract_call(predicate: StacksPredicate, expected_appl expire_after_occurrence: None, capture_all_events: None, decode_clarity_values: None, - include_contract_abi: false, + include_contract_abi: None, predicate: predicate, action: HookAction::Noop, enabled: true, @@ -657,7 +657,7 @@ fn test_stacks_hook_action_noop() { expire_after_occurrence: None, capture_all_events: None, decode_clarity_values: None, - include_contract_abi: false, + include_contract_abi: None, predicate: StacksPredicate::Txid(ExactMatchingRule::Equals( "0xb92c2ade84a8b85f4c72170680ae42e65438aea4db72ba4b2d6a6960f4141ce8".to_string(), )), @@ -715,7 +715,7 @@ fn test_stacks_hook_action_file_append() { expire_after_occurrence: None, capture_all_events: None, decode_clarity_values: Some(true), - include_contract_abi: false, + include_contract_abi: None, predicate: StacksPredicate::Txid(ExactMatchingRule::Equals( "0xb92c2ade84a8b85f4c72170680ae42e65438aea4db72ba4b2d6a6960f4141ce8".to_string(), )), diff --git a/components/chainhook-sdk/src/chainhooks/types.rs b/components/chainhook-sdk/src/chainhooks/types.rs index 3041c6457..c26566d98 100644 --- a/components/chainhook-sdk/src/chainhooks/types.rs +++ b/components/chainhook-sdk/src/chainhooks/types.rs @@ -363,7 +363,7 @@ impl StacksChainhookFullSpecification { capture_all_events: spec.capture_all_events, decode_clarity_values: spec.decode_clarity_values, expire_after_occurrence: spec.expire_after_occurrence, - include_contract_abi: spec.include_contract_abi.unwrap_or(false), + include_contract_abi: spec.include_contract_abi, predicate: spec.predicate, action: spec.action, enabled: false, @@ -688,7 +688,7 @@ pub struct StacksChainhookSpecification { pub capture_all_events: Option, #[serde(skip_serializing_if = "Option::is_none")] pub decode_clarity_values: Option, - pub include_contract_abi: bool, + pub include_contract_abi: Option, #[serde(rename = "predicate")] pub predicate: StacksPredicate, pub action: HookAction,