input-output-hk / voting-tools

Apache License 2.0
17 stars 10 forks source link

Improve output format of voter-registration #28

Closed sevanspowell closed 2 years ago

sevanspowell commented 2 years ago

Issue number

ADP-1298

Overview

Change the voter-registration tool to only output TxMetadata, in JSON or CBOR formats. This metadata can then be attached to a transaction for submission. This is a more flexible approach, and frees the voter-registration tool from having to calculate fees, etc.

Also, force the user to specify a slot number, instead of querying the local node. This allows vote registration transaction metadata to be created offline.

I'm making these changes based on the following reasoning:

sevanspowell commented 2 years ago

@gitmachtl How do you feel about something like this?

gitmachtl commented 2 years ago

@gitmachtl How do you feel about something like this?

yeah, love it. i think a simple metadata output on json and cbor is perfect for this usecase. this tool is already "only" for advanced users, using it on the cli for integration. i like it.

gitmachtl commented 2 years ago

The same solution using the hw-cli includes a nonce parameter, i am not sure if that nonce parameter was ever used for the voting/registration process in any way. It is normally set to be the current slotHeight, which can also be calculated offline. But i don't know if this parameter is really needed.

sevanspowell commented 2 years ago

Ah thanks @gitmachtl. I didn't quite understand the "nonce" parameter in your original request. Looking at hw-cli now, I believe it's just another name for the slot number (see here: https://github.com/vacuumlabs/cardano-hw-cli/blob/f5616cede90b43cbdd5097f01353ebe3713a06ce/src/command-parser/parserConfig.ts#L236). I think hw-cli and voting-tools are just using different words for the same thing.

The slot number is used here to prevent replay attacks. Is being able to provide a custom slot height (which I've enabled in this PR) sufficient?

Thanks for your input as always, apologies this wasn't addressed sooner.

gitmachtl commented 2 years ago

The slot number is used here to prevent replay attacks. Is being able to provide a custom slot height (which I've enabled in this PR) sufficient?

yes, thats the same parameter. i just scrolled over the commit changes and missed that. all good 😄

sevanspowell commented 2 years ago

Great!

gitmachtl commented 2 years ago

just compiled and tested it, looks good. json and cbor output is working. will this be an extra binary version or do you integrate or replace it in the main one?

image

sevanspowell commented 2 years ago

just compiled and tested it, looks good. json and cbor output is working. will this be an extra binary version or do you integrate or replace it in the main one?

image

Awesome!

What do you mean extra binary version? Like do I plan to include this in v0.2.0.0 (If so, yes I do plan to include this!)?

gitmachtl commented 2 years ago

What do you mean extra binary version? Like do I plan to include this in v0.2.0.0 (If so, yes I do plan to include this!)?

ok that is awesome, just what we need.

sevanspowell commented 2 years ago

I've got to head off for the day, but I'll pick this up tomorrow - just some documentation to finish.