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

devtools/onion: fixed argument passing for generate and decode methods #3250

Merged
merged 4 commits into from
Nov 14, 2019

Conversation

remyers
Copy link
Contributor

@remyers remyers commented Nov 8, 2019

Fixed 'generate' and 'decode' methods to properly parse argv arguments.
Fixed so methods correctly use the associated data set by the --assoc-data flag.

I also added some examples to -h to better show how these methods work with real onion data.

Introduced a behavioral change to read the onion from a file instead of stdin; this makes it easier to debug in some IDEs (eg. vscode).

devtools/onion.c Outdated
@@ -109,7 +109,7 @@ static void do_generate(int argc, char **argv,
u8 *serialized = serialize_onionpacket(ctx, res);
if (!serialized)
errx(1, "Error serializing message.");
printf("%s\n", tal_hex(ctx, serialized));
printf("%s", tal_hex(ctx, serialized));
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this is a bit awkward if you run it on the cmdline though?

Can we instead fix it up on the input side?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point; I've rebased to leave the trailing newline but instead trim whitespace with 6a078de . Is this better?

@rustyrussell rustyrussell added this to the 0.7.4 milestone Nov 13, 2019
@rustyrussell
Copy link
Contributor

Nice work!

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 6a078de

@rustyrussell rustyrussell merged commit 2821187 into ElementsProject:master Nov 14, 2019
@remyers
Copy link
Contributor Author

remyers commented Nov 14, 2019

Thanks! and ht to @willcl-ark for the testing that lead to these fixes.

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.

2 participants