Skip to content

Add new return fields from later versions to v17 types #271

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

Conversation

jamillambert
Copy link
Collaborator

Some of the methods have new return fields in later versions. Add the fields to the existing types in v17 as options and update the errors, models and into functions.

Patches:

  • Remove an option from a non optional field.
  • Add fields to decodescript, update the models, errors and into functions.
  • Reorder the list in logging alphabetically, no other changes.
  • Add fields to logging, remove redefinitions for later versions and reexport v17 for all versions.
  • Add field to scriptpubkey.
  • Add fields to gettransaction, add the new returned type LastProcessedBlock, update the models, errors and into functions.

Returned field `type` from `decodescript` is not optional for v17 to
v29.

Remove the Option and make it a String.
New return fields added in v22 and v23.

Add the new fields to the model and v17 type and into function.
The core help gives a list in alphabetical order.

Change the list here to match and make reading it easier.
Fields were added in v22, v23, v25, v26.

Remove the redefinitions in version specific folders and add the types
as options to the v17 definition.
A return field `desc` was added in v23.

Add it as an option to the struct.
Fields were added in v23, v24, v26, v29.

Add the new return fields as options to the v17 definition.

Add the new LastProcessedBlock type, model, error and into function.

Add GetTransaction error variants, update the model and into function.
@jamillambert jamillambert force-pushed the 0625-v22up-missing-fields-to-v17 branch from 4cd04d1 to c00c257 Compare June 25, 2025 14:56
@jamillambert
Copy link
Collaborator Author

jamillambert commented Jun 25, 2025

This is a deviation from what was done before, adding the fields to v17 rather than redefining the whole type for a later version. It is significantly easier, in particular if there is just a single return field added in a later version.

I did it as a kind of proof of concept to see if we should move in this direction.

pub selectcoins: bool,
pub reindex: bool,
pub bench: bool,
pub blockstorage: Option<bool>, // v23 and later only
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR At this stage I don't want to make changes like this because it invalidates the whole design of the crate.

I accidentally did this same thing once but I came to the conclusion that doing so totally invalidates the whole design of the crate. The whole point of v17::Foo is that one knows that this is the exact shape of a Foo returned by Bitcoin Core v17. If one wants to not think about the version and check all the optional fields then one can first convert to a model type and do so. The central claim of this repo is that these use cases are different and keeping them separate will lead to better software. This was an intuitive leap and only time will tell if I was correct.

@jamillambert
Copy link
Collaborator Author

Done separately for every version as suggested in #272.

@jamillambert jamillambert deleted the 0625-v22up-missing-fields-to-v17 branch July 4, 2025 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants