-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add support to tapyrus-setup sign and createsig commands to sign xfield #169
Add support to tapyrus-setup sign and createsig commands to sign xfield #169
Conversation
add test for xfield signature generation
a193006
to
6c8c763
Compare
Add tests for federation toml
@@ -1,21 +1,19 @@ | |||
[[federation]] | |||
block-height = 0 | |||
threshold = 3 | |||
aggregated-public-key = "030d856ac9f5871c3785a2d76e3a5d9eca6fcce70f4de63339671dfb9d1f33edb0" | |||
threshold = 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change eliminates the effect of changing the threshold, so please make a test that also changes the threshold.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is added in a new test test_load_federations_threshold_change
too.
Co-authored-by: shigeyuki azuchi <[email protected]>
src/cli/setup/compute_sig.rs
Outdated
} else if xfield != XField::None { | ||
xfield.signature_hash().unwrap().to_vec() | ||
} else { | ||
return Err(Error::InvalidArgs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this line be executed?
I think this line is unreachable, as the same check is on line 204.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. this is unreachable. I'll fix it.
fn valid_federation_maxblocksize() -> Federation { | ||
let mut federation = valid_federation(); | ||
|
||
federation.xfield = XField::MaxBlockSize(400000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any restrictions on the maximum or minimum MaxBlockSize?
If a small value (maybe 170bytes) is specified, will TapyrusCore reject the block?
According to https://github.com/chaintope/tapyrus-core/blob/master/doc/tapyrus/signedblocks.md, the minimum Block Header is 170byte.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any restrictions on the maximum or minimum MaxBlockSize?
Tapyrus Core requires the minimum block size to be 1000 bytes. There is no maximum limit other than the 4 byte capacity allowed.
If a small value (maybe 170bytes) is specified, will TapyrusCore reject the block?
Yes, Tapyrus Core does not allow max block size in xfield to be set below 1000 bytes. Also, BlockAssembler in Tapyrus Core uses the same minimum limit. However if a smaller size valid block is created outside of Tapyrus Core like using tools like our test framework, there is no size restriction. The block is accepted by the node after validation.
This PR adds the command line support to generate a federation signature to make an xfield change.
commands are used to create the xfield signature
Use XFieldHash type hash the serialized xfield to generate the signature
Add tests to verify signing xfield instead of block. A valid signature was generated using the commands in a 3 signer, 2 threshold setup and is added to the unit test.