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

json: Use i64 where possible #40

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Sep 12, 2024

The Bitcoin Core JSONRPC API has fields marked as 'numeric'. It is not obvious what Rust type these fields should be.

We want the version specific JSON types to just work (TM).

  1. We use an i64 because its the biggest signed integer on "common" machines.
  2. We use a signed integer because Core sometimes returns -1.

Some fields are then converted to rust-bitcoin types that expect a u64 (eg sats), these are left as u64 and we just hope Core never returns -1 for any of them.

@tcharding
Copy link
Member Author

Props to @stevenroose for the pointing out the issue and the suggested fix. It was an off the cuff suggestions so any problems created are my own.

@tcharding tcharding changed the title json: Use u64 where possible json: Use i64 where possible Sep 12, 2024
@tcharding tcharding changed the title json: Use i64 where possible json: Use i64 where possible Sep 12, 2024
The Bitcoin Core JSONRPC API has fields marked as 'numeric'. It is not
obvious what Rust type these fields should be.

We want the version specific JSON types to just work (TM).

1. We use an `i64` because its the biggest signed integer on "common"
   machines.
2. We use a signed integer because Core sometimes returns -1.

Some fields are then converted to `rust-bitcoin` types that expect a
`u64` (eg sats), these are left as `u64` and we just hope Core never
returns -1 for any of them.
@tcharding tcharding merged commit af6db57 into rust-bitcoin:master Sep 18, 2024
31 checks passed
@tcharding tcharding deleted the 09-12-use-i64 branch October 24, 2024 03:10
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.

1 participant