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

Nits and fixups #5016

Merged
merged 7 commits into from
Feb 7, 2022
Merged

Conversation

niftynei
Copy link
Contributor

@niftynei niftynei commented Jan 21, 2022

Things found while working on the accountant plugin.

I have no idea how the wallet error made it past previous checks. Whoops.

The listchannels addition of the funding_outnum is extremely useful for backfilling missed channel open events.

This wasn't used anywhere, so we remove it.
Add missing field to first write
@niftynei niftynei requested a review from cdecker as a code owner January 21, 2022 21:55
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 087ca31

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ack 087ca31

The CI error looks strange, and maybe it needs another run!

@cdecker
Copy link
Member

cdecker commented Jan 24, 2022

Seems the JSON-RPC schema needs to be updated:

Additional properties are not allowed ('funding_outnum' was unexpected)

@niftynei niftynei force-pushed the nifty/acct-fixups branch 2 times, most recently from fae0f32 to dd58182 Compare January 24, 2022 17:31
Comment on lines 238 to 241
"funding_outnum": {
"type": "u32",
"description": "The 0-based output number of the funding transaction which opens the channel"
},
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo Jan 24, 2022

Choose a reason for hiding this comment

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

Mh from CI there is a formatting problem here, maybe a make check-fmt-schemas will call jq to auto-formatting the json file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep that was the issue for this test. Nice catch!

Changelog-Added: JSONRPC: `listchannels` now includes the `funding_outnum`
The money moved into an external account here.
There is no "wallet_lib_headers" variable in wallet/Makefile
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK 86c9958

@rustyrussell rustyrussell merged commit 9ebdb71 into ElementsProject:master Feb 7, 2022
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.

4 participants