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

Fix off-by-some --height-limit bug #526

Merged
merged 19 commits into from
Sep 19, 2022
Merged

Fix off-by-some --height-limit bug #526

merged 19 commits into from
Sep 19, 2022

Conversation

casey
Copy link
Collaborator

@casey casey commented Sep 13, 2022

Fixes #517. We were reading the height using self.height(), which opens a new read transaction, which will return the old height, until the current write transaction is committed.

Instead, read the height from the open write transaction, and increment it every time we index a block.

This should be ready to merge once I add a test.

todo:

  • remove transactions from dummy blocks?
  • #[rpc] -> #[jsonrpc_derive::rpc]
  • consider parsing options directly
  • do we need Default impls?
  • fix localhost address
  • convert height_limit to u64
  • convert --height-limit integration test into unit test
  • can i get rid of fallible conversion?
  • make height stuff coherent
  • reorder BicoinRpcServerHandle

@casey casey marked this pull request as ready for review September 17, 2022 01:40
@casey casey enabled auto-merge (squash) September 17, 2022 02:07
@casey casey disabled auto-merge September 17, 2022 18:27
src/test.rs Show resolved Hide resolved
tests/state.rs Outdated Show resolved Hide resolved
tests/state.rs Outdated Show resolved Hide resolved
@casey casey enabled auto-merge (squash) September 19, 2022 20:07
@casey casey merged commit 83700d0 into master Sep 19, 2022
@casey casey deleted the fix-off-by-some branch September 19, 2022 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Off by some bug in -height-limit
2 participants