-
Notifications
You must be signed in to change notification settings - Fork 40
Add support for Bitcoin Core 30.0 #387
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
Two of the methods are listed twice in v27, v28 and v29. Remove the duplicates.
Add all the required files, features and update existing code to support Bitcoin Core v30. New files are coppies of v29.
Update v30 files which are copies of the v29 to apply the changes made to Core between v29 and v30. Add TODO to all v30 methods that have changes and need updating. Feature gate the tests for methods that have been dropped or are TODO in v30.
c64ee5a to
e2507f4
Compare
tcharding
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK e2507f4
|
|
||
| # Enable the same feature in `node` and the version feature here. | ||
| # All minor releases of the latest three versions. | ||
| # All minor releases of the latest four versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Novel idea, increase this number so as not to have to make changes to the 4th oldest release. Are you planning on dropping this back to 3 or do you think we leave it as is for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't see the reason to remove them? No effort to keep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a policy thing, if we keep this and 27.x comes out we have to add support for it. Although this moving window has something annoying about it for downstream but I can't remember right now what it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I'm a goose, we have to anyway because we say we support the latest. hmmm not sure what to do, lets leave it until we think of something better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I wasn't sure what effect removing 27.0 and 27.1 was on downstream. Mostly why I left them in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its a policy thing, if we keep this and 27.x comes out we have to add support for it.
The comment below says # Only the latest minor version for older versions. which would suggest we have to anyway. I guess we don't want this for the older versions and should change the wording.
EDIT: 😊 I see you wrote the same thing above.
|
F**k you learn fast. Really nice separation of concerns in this PR. By that I mean what to change and what to leave for later. |
2343534 Update listwalletdir for v30 changes (Jamil Lambert, PhD) cb73a83 Update getwalletinfo for v30 changes (Jamil Lambert, PhD) 86c40a4 Document that migratewallet is untested in v30 (Jamil Lambert, PhD) 4dbd863 Add deprecated flag to v30 settxfee test (Jamil Lambert, PhD) Pull request description: There are four remaining RPCs that have changes in v30 that need to be implemented: - `settxfee` is deprecated in v30 and removed in v31. Feature gate the test to v30 and below and for v30 add the required `-deprecatedrpc=settxfee` flag. Remove TODO from the types table. - In v30 it is no longer possible to create a legacy wallet. This means that `migratewallet` cannot be tested using only v30. There are no changes to the RPC in v30 so the v29 test is still valid. Add a comment to the types table and test. - `getwalletinfo` had some return fields removed in v30 and a new flags field was added. Update the model to make the removed fields options and add the new flags. Update all the into functions. Redefine the type, error and into function for v30. Remove the TODO from the types table. Remove the `v29_and_below` feature gate from the test. - `listwalletdir` has a new `warnings` field was added in v30. Redefine the type for v30, there is no model. Remove the TODO from the types table. Remove the `v29_and_below` feature gate from the test. Together with #387, #388, #409 and #410 Closes #384 ACKs for top commit: tcharding: ACK 2343534 Tree-SHA512: 788e6150778dc722c7db79e63240d4c82d3ceca59ec859edd516d1ea8860671754be3072f4b5bdd391dac919bba6a61935c99df4bb3b4325cf981271ef4e87bf
Add all the necessary files and changes to support Core v30.
RPCs that were dropped in v30 have been removed.
New and changed RPCs between v29 and v30 have not been implemented in this PR and are TODO in
types.Patches are:
verify/src/method/v30.rsis a copy of v29 with the label changed to v30.types/src/v30/mod.rsis a copy of v29 with the exports of the new v29 types moved to a reexport section under v29, no other changes.