Skip to content

Address all TODO in v23 #301

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Address all TODO in v23 #301

wants to merge 5 commits into from

Conversation

jamillambert
Copy link
Collaborator

@jamillambert jamillambert commented Jul 16, 2025 •
edited
Loading

Go through all the TODO in the v23 types table. Add all the missing RPCs.

  • Add getblockfrompeer macro and test
  • Add restorewallet struct, macro and test
  • Add newkeypool macro and test
  • Add getdeploymentinfo struct, model and test
  • Run the formatter

@jamillambert
Copy link
Collaborator Author

GetDeploymentInfo has a lot of return fields, and only pub hash: BlockHash that is modelled, all the sub structs are also in the model since they are a different type, even though all the fields are identical to the ones in types. I wasn't sure if this is correct or if there is a better way, also not sure about the error for it due to the multi layered into_model functions that don't really do anything.

@jamillambert jamillambert marked this pull request as draft July 16, 2025 20:11
@jamillambert
Add getblockfrompeer macro and test
6ffbf1d 
The types table said version + model but an empty json object is
returned.

Update the table to `returns nothing`.

Add a client macro that returns `Ok` if an empty json object is
returned.

Add a test where the RPC succeeds and returns an empty object.
@jamillambert
Add restorewallet struct, macro and test
5429040 
Add to the existing `backup_wallet` test, since the backup is needed
before restoring.

Split out the test into backup_and_restore_wallet()
to make it clear this is for multiple RPCs.

Add an empty test for restorewallet with a comment saying it is tested
as part of backupwallet.
@jamillambert
Add newkeypool macro and test
869f0c4 
newkeypool returns null. Add a client macro and test to check that it
does.

Update the types table to say it returns nothing.
@jamillambert
Add getdeploymentinfo struct, model and test
9e6f836 
Getdeploymentinfo returns a chain of json objects. Add structs for all
of them and a model with an into function.

Add a client macro and test.
@jamillambert
Run the formatter
828454a 
Reordering of the reexports only. No other changes.
@jamillambert
Copy link
Collaborator Author

Rebased on master, no changes to the commits.

@jamillambert jamillambert marked this pull request as ready for review July 21, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
C-client C-integration-test C-types C-verify
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@jamillambert