Skip to content
This repository was archived by the owner on Nov 26, 2024. It is now read-only.

Do a bunch of minor improvements #31

Merged
merged 11 commits into from
Sep 6, 2024
Merged

Do a bunch of minor improvements #31

merged 11 commits into from
Sep 6, 2024

Conversation

tcharding
Copy link
Member

Remove FIXMEs and things like that, non of this is controversial.

The `BlockHash` type is 32 bytes and implements `Copy`, we should pass
it by value not by reference.
Convert `Option<Result<T, E>>` to ``Result<Option<T>, E>` by using the
`transpose` combinator - TIL.
The `v17::GetNetworkInfo` does not have connections in and out, only
total. We should not have these fields until we support a version of
Core that does have these fields.
This is kruft from an earlier design; currently we only support v17
`sendtoaddress` which only responds with the txid.
This is indeed the correct way to get `collect` to transform a list of
`Result<T, E>` to a `Result<Vec<T>, E>`.
The integration tests will catch this if it is wrong.
We want to provide all shapes of JSON data so having two explicit
functions is a reasonable way, I can't think of a better one right now.
I may have got mixed up with `getblockchaininfo` when I wrote this
FIXME, we only support v17 `getnetworkinfo` and the `warnings` field is
correctly named (and documented).
Investigate and remove the fixme, leave a more terse comment incase
someone stumbles upon this later.
This code is verbose but trivial, lets leave it in.
@tcharding tcharding merged commit 9d71d32 into master Sep 6, 2024
31 checks passed
@tcharding tcharding deleted the 06-09-improve branch September 6, 2024 03:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant