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

Stacks block commitments incorrectly assign change outputs as pox transfers #496

Closed
aulneau opened this issue Feb 13, 2024 · 5 comments · Fixed by #499
Closed

Stacks block commitments incorrectly assign change outputs as pox transfers #496

aulneau opened this issue Feb 13, 2024 · 5 comments · Fixed by #499
Labels

Comments

@aulneau
Copy link

aulneau commented Feb 13, 2024

Describe the bug

For the block commitments predicate, burn related pox blocks are incorrectly assigning change outputs as pox transfer events:

       "stacks_operations": [
          {
            "block_committed": {
              "block_hash": "0x5b7e468df57b9d0208b67ffd3bc145a48e200270afb94fc8b11d910760276a01",
              "mining_address_post_commit": null,
              "mining_sats_left": 0,
              "pox_cycle_index": 74,
              "pox_cycle_length": 2200,
              "pox_cycle_position": 997,
              "pox_sats_burnt": 501000,
              "pox_sats_transferred": [
                {
                  "amount": 626173985,
                  "recipient_address": "1UiW61F9xnBnnC1J69aPJhFm3ZXvZEkCm"
                }
              ]
            }
          }
        ]

Here is the entire output for a block commitment event for block 829847, and the related transaction: https://mempool.space/tx/151bc0d6ced51f3f7675f0caa60769543f2ec5962f0fe271c14b8c5feca466cf

https://gist.github.com/aulneau/f612837d61cbcc8e12d6d0a4c9ef8c63

seems possible this condition is incorrect:

.is_consensus_rewarding_participants_at_block_height(block_height)

or maybe this logic?

if pox_address_2.to_string().eq(&pox_config.get_burn_address()) {
pox_sats_burnt += pox_output_2.value.to_sat();
} else {
pox_sats_transferred.push(PoxReward {
recipient_address: pox_address_2.to_string(),
amount: pox_output_2.value.to_sat(),
});

@lgalabru
Copy link
Contributor

Thanks @aulneau, we're looking into this.

@MicaiahReid
Copy link
Contributor

Hey @aulneau, great find on this bug!

I have a fix out in PR #499. Would you be able to checkout the branch fix-block-commitment, rebuild Chainhook, and confirm that things are working as expected?

@aulneau
Copy link
Author

aulneau commented Feb 14, 2024

Hey @aulneau, great find on this bug!

I have a fix out in PR #499. Would you be able to checkout the branch fix-block-commitment, rebuild Chainhook, and confirm that things are working as expected?

Unfortunately I do not have a bitcoin node set up -- I'm using the hiro platform for any bitcoin specific predicates

@MicaiahReid
Copy link
Contributor

MicaiahReid commented Feb 14, 2024

@aulneau @lgalabru I've deployed this fix to our dev infrastructure. I made this predicate:

{
  "name": "Test Block Commitment Fix",
  "uuid": "08652ee3-733d-4a27-9442-09b0f142b501",
  "chain": "bitcoin",
  "version": 1,
  "networks": {
    "mainnet": {
      "if_this": { "scope": "stacks_protocol", "operation": "block_committed" },
      "end_block": 829848,
      "then_that": {
        "http_post": {
          "url": "...",
          "authorization_header": ""
        }
      },
      "start_block": 829847,
      "include_proof": false,
      "include_inputs": true,
      "include_outputs": true,
      "include_witness": false,
      "expire_after_occurrence": null
    }
  }
}

and got this output for the block commitment:

"stacks_operations": [
    {
      "block_committed": {
        "block_hash": "0x5b7e468df57b9d0208b67ffd3bc145a48e200270afb94fc8b11d910760276a01",
        "mining_address_post_commit": "1UiW61F9xnBnnC1J69aPJhFm3ZXvZEkCm",
        "mining_sats_left": 626173985,
        "pox_cycle_index": 74,
        "pox_cycle_length": 2200,
        "pox_cycle_position": 997,
        "pox_sats_burnt": 501000,
        "pox_sats_transferred": []
      }
    }
]

We should have this fix pushed to the platform on the next release!

MicaiahReid added a commit that referenced this issue Feb 14, 2024
### Description

Fixes #496 

### Checklist

- [X] All tests pass
- [ ] Tests added in this PR (if applicable)
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in DevTools Feb 14, 2024
MicaiahReid added a commit that referenced this issue Feb 14, 2024
### Description

Fixes #496 

### Checklist

- [X] All tests pass
- [ ] Tests added in this PR (if applicable)
github-actions bot pushed a commit that referenced this issue Feb 14, 2024
## [1.3.1](v1.3.0...v1.3.1) (2024-02-14)

### Bug Fixes

* add event index to transaction events ([#495](#495)) ([d67d1e4](d67d1e4)), closes [#417](#417) [#387](#387)
* correctly determine PoX vs PoB block commitments ([#499](#499)) ([50dd26f](50dd26f)), closes [#496](#496)
Copy link

🎉 This issue has been resolved in version 1.3.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

vabanaerytk added a commit to vabanaerytk/chainhook that referenced this issue Aug 7, 2024
## [1.3.1](hirosystems/chainhook@v1.3.0...v1.3.1) (2024-02-14)

### Bug Fixes

* add event index to transaction events ([#495](hirosystems/chainhook#495)) ([2d2760d](hirosystems/chainhook@2d2760d)), closes [#417](hirosystems/chainhook#417) [#387](hirosystems/chainhook#387)
* correctly determine PoX vs PoB block commitments ([#499](hirosystems/chainhook#499)) ([6816e76](hirosystems/chainhook@6816e76)), closes [#496](hirosystems/chainhook#496)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants