Skip to content

Commit

Permalink
fix: make include_contract_abi optional (#432)
Browse files Browse the repository at this point in the history
### 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)
  • Loading branch information
MicaiahReid authored Oct 9, 2023
1 parent da500d7 commit 947b11e
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 11 deletions.
2 changes: 1 addition & 1 deletion components/chainhook-sdk/src/chainhooks/stacks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions components/chainhook-sdk/src/chainhooks/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()),
),
Expand All @@ -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(),
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
)),
Expand Down Expand Up @@ -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(),
)),
Expand Down
4 changes: 2 additions & 2 deletions components/chainhook-sdk/src/chainhooks/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -688,7 +688,7 @@ pub struct StacksChainhookSpecification {
pub capture_all_events: Option<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
pub decode_clarity_values: Option<bool>,
pub include_contract_abi: bool,
pub include_contract_abi: Option<bool>,
#[serde(rename = "predicate")]
pub predicate: StacksPredicate,
pub action: HookAction,
Expand Down

0 comments on commit 947b11e

Please sign in to comment.