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

tools: update CSV generator to output fields for tlv's and subtypes #597

Closed
wants to merge 5 commits into from

Conversation

niftynei
Copy link
Collaborator

@niftynei niftynei commented Apr 2, 2019

v1.1 message formats add new types (tlv's and subtypes). this updates
the csv extraction tool to appropriately output them

v1.1 message formats add new types (tlv's and subtypes). this updates
the csv extraction tool to appropriately output them
niftynei added a commit to niftynei/lightning that referenced this pull request Apr 3, 2019
Updated to match what the CSV generator in the RFC repo actually
outputs, see lightning/bolts#597
rustyrussell pushed a commit to ElementsProject/lightning that referenced this pull request Apr 3, 2019
Updated to match what the CSV generator in the RFC repo actually
outputs, see lightning/bolts#597
Copy link
Collaborator

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

It'd be nice to have the output form listed somewhere too (it's currently pretty undocumented).

else:
print("{},{}".format(
match.group('name'),
str(match.group('value') or '$')), file=output)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think subtypes should just be a single field, ie. no value. Rather than a double field with a magic '$' sign?

niftynei and others added 3 commits April 8, 2019 19:25
subtypes used to use a magic '$' to designate that they
were embedded; now we print them without magic.

previous:
input_info,$
input_info,0,xxx

now:
input_info
input_info,0,xxx

previous:
funding_message,2,field,8
funding_message,10,num_inputs,2
funding_message,12,input_info,$num_inputs

now:
funding_message,2,field,8
funding_message,10,num_inputs,2
funding_message,12,input_info,num_inputs*input_info
For example, @cdecker used a FRAME_SIZE constant in lightning#593 which broke
parsing.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
Copy link
Collaborator

OK, just pushing one unrelated tweak, which fixes parsing of #604 and #593

@rustyrussell rustyrussell added the Meeting Discussion Raise at next meeting label May 6, 2019
@cdecker
Copy link
Collaborator

cdecker commented May 13, 2019

ACK 33ff459

1. Uses 'tlvs' for the "length" of tlv streams.
2. Allow - inside length of fields, since it's useful given an implied tlv_len.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
Copy link
Collaborator

I think this was rolled into #622 ?

@rustyrussell
Copy link
Collaborator

Closed in favor of #622 which was merged.

@niftynei niftynei removed the Meeting Discussion Raise at next meeting label Jul 22, 2019
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.

3 participants