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

Chainhook should not error when end_block is missing; should just abort scan #477

Closed
MicaiahReid opened this issue Jan 23, 2024 · 1 comment · Fixed by #489
Closed

Chainhook should not error when end_block is missing; should just abort scan #477

MicaiahReid opened this issue Jan 23, 2024 · 1 comment · Fixed by #489
Assignees
Labels

Comments

@MicaiahReid
Copy link
Contributor

Here and here, we throw an error if the end_block is not provided (or if we can't find blocks in the database).

I believe we should just revert to streaming mode in this case.

@github-project-automation github-project-automation bot moved this to 🆕 New in DevTools Jan 23, 2024
@MicaiahReid MicaiahReid moved this from 🆕 New to 📋 Backlog in DevTools Jan 23, 2024
@MicaiahReid MicaiahReid self-assigned this Feb 8, 2024
MicaiahReid added a commit that referenced this issue Feb 8, 2024
### Description

Previously, the scanning threads had some validation for the
`start_block` and `end_block` that was incorrect. This PR introduces
validation that does the following:
- We now allow `start_block` to be omitted by the user and we default to
0
- If there are no blocks in the database, we abort the scan and go to
streaming mode rather than erroring (fixes #477)
- If the user provides an `end_block`, we validate that it is greater
than the `start_block`
- If the `start_block` is greater than chain tip, we abort the scan and
go to streaming mode rather than erroring (fixes #464)

This PR also adds some validation to the `BlockHeights` class.
Previously, it was possible to overload the
`BlockHeights::BlockRange(start_block, end_block)` function to allocate
a lot of memory into an empty array. We now have limits on this class.
However, due to the above validation, it should not be possible to pass
through parameters that reach theses limits (with the current usage of
the class) until a chain height is up to `1_000_000`.

---

### Checklist

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

Previously, the scanning threads had some validation for the
`start_block` and `end_block` that was incorrect. This PR introduces
validation that does the following:
- We now allow `start_block` to be omitted by the user and we default to
0
- If there are no blocks in the database, we abort the scan and go to
streaming mode rather than erroring (fixes #477)
- If the user provides an `end_block`, we validate that it is greater
than the `start_block`
- If the `start_block` is greater than chain tip, we abort the scan and
go to streaming mode rather than erroring (fixes #464)

This PR also adds some validation to the `BlockHeights` class.
Previously, it was possible to overload the
`BlockHeights::BlockRange(start_block, end_block)` function to allocate
a lot of memory into an empty array. We now have limits on this class.
However, due to the above validation, it should not be possible to pass
through parameters that reach theses limits (with the current usage of
the class) until a chain height is up to `1_000_000`.

---

### Checklist

- [x] All tests pass
- [x] Tests added in this PR (if applicable)
github-actions bot pushed a commit that referenced this issue Feb 8, 2024
## [1.3.0](v1.2.1...v1.3.0) (2024-02-08)

### Features

* optionally serve Prometheus metrics ([#473](#473)) ([67a38ac](67a38ac))

### Bug Fixes

* adjust ordinal_number entry in ts client inscription transfer event, add new reveal data ([#476](#476)) ([28bf5c4](28bf5c4))
* remove early return for event evaluation ([#484](#484)) ([98f9e86](98f9e86)), closes [#469](#469)
* remove unreachable panic; return instead ([#490](#490)) ([abe0fd5](abe0fd5))
* use cli feature for `cargo chainhook-install` ([#486](#486)) ([32f4d4e](32f4d4e))
* validate predicate `start_block` and `end_block` ([#489](#489)) ([e70025b](e70025b)), closes [#477](#477) [#464](#464)
Copy link

github-actions bot commented Feb 8, 2024

🎉 This issue has been resolved in version 1.3.0 🎉

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.0](hirosystems/chainhook@v1.2.1...v1.3.0) (2024-02-08)

### Features

* optionally serve Prometheus metrics ([#473](hirosystems/chainhook#473)) ([1a3e356](hirosystems/chainhook@1a3e356))

### Bug Fixes

* adjust ordinal_number entry in ts client inscription transfer event, add new reveal data ([#476](hirosystems/chainhook#476)) ([fe7ef78](hirosystems/chainhook@fe7ef78))
* remove early return for event evaluation ([#484](hirosystems/chainhook#484)) ([d0e2f60](hirosystems/chainhook@d0e2f60)), closes [#469](hirosystems/chainhook#469)
* remove unreachable panic; return instead ([#490](hirosystems/chainhook#490)) ([c7894ac](hirosystems/chainhook@c7894ac))
* use cli feature for `cargo chainhook-install` ([#486](hirosystems/chainhook#486)) ([206cb17](hirosystems/chainhook@206cb17))
* validate predicate `start_block` and `end_block` ([#489](hirosystems/chainhook#489)) ([85f3e71](hirosystems/chainhook@85f3e71)), closes [#477](hirosystems/chainhook#477) [#464](hirosystems/chainhook#464)
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.

1 participant