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

doc: fix listnodes schema, doc. #4750

Merged
merged 2 commits into from
Sep 3, 2021

Conversation

rustyrussell
Copy link
Contributor

Shows the dangers of "additionalProperties": true. We didn't have an else
clause, so our incorrect (and, with DF, incomplete!) schema was accepted.

I despair of getting anyone else to write a decent schema with these
semantics :(

Changelog-Fixed: doc: listnodes fields now correctly documented.
Signed-off-by: Rusty Russell rusty@rustcorp.com.au

@rustyrussell rustyrussell added this to the v0.10.2 milestone Aug 31, 2021
@cdecker
Copy link
Member

cdecker commented Aug 31, 2021

Seems to be missing a required key somewhere:

fromschema doc/lightning-listnodes.7.md
Traceback (most recent call last):
  File "tools/fromschema.py", line 276, in <module>
    main(parsed_args.schemafile, parsed_args.markdownfile)
  File "tools/fromschema.py", line 266, in main
    generate_from_schema(schema)
  File "tools/fromschema.py", line 234, in generate_from_schema
    output_members(sub)
  File "tools/fromschema.py", line 193, in output_members
    output_members(ifclause['then'], indent + '  ')
  File "tools/fromschema.py", line 140, in output_members
    if p in sub['required']:
KeyError: 'required'
make: *** [doc/Makefile:102: doc/lightning-listnodes.7.md] Error 1

- **funding_weight** (u32): the onchain weight you'll have to pay for a lease
- **channel_fee_max_base_msat** (msat): the maximum base routing fee this node will charge during the lease
- **channel_fee_max_proportional_thousandths** (msat): the maximum proportional routing fee this node will charge during the lease (in thousandths, not millionths like channel_update)
- **compact_least** (hex, optional): the lease as represented in the node_announcement
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- **compact_least** (hex, optional): the lease as represented in the node_announcement
- **compact_lease** (hex, optional): the lease as represented in the node_announcement

"type": "msat",
"description": "the maximum proportional routing fee this node will charge during the lease (in thousandths, not millionths like channel_update)"
},
"compact_least": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"compact_least": {
"compact_lease": {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gah, this was clearly untested... :(

Shows the dangers of "additionalProperties": true.  We didn't have an else
clause, so our incorrect (and, with DF, incomplete!) schema was accepted.

I despair of getting anyone else to write a decent schema with these
semantics :(

Changelog-Fixed: doc: listnodes fields now correctly documented.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker
Copy link
Member

cdecker commented Sep 2, 2021

Regenerated the docs since it was failing with a different source hash.

Copy link
Collaborator

@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 a68cf25

@rustyrussell rustyrussell merged commit 7e43109 into ElementsProject:master Sep 3, 2021
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