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

TIP-29 Milestone Payload (Stardust) #69

Merged
merged 21 commits into from
Sep 8, 2022
Merged

Conversation

Wollac
Copy link
Contributor

@Wollac Wollac commented Mar 25, 2022

Description

This PR slightly modifies the original Milestone Payload definition by removing the Keys from the Milestone Essence in favor of regular Signature Blocks consistent with TIP-20.

This supersedes the Milestone Payload introduced in TIP-8.

View

Rendered Document

@Wollac Wollac changed the title TIP-30 Milestone Payload with Keys remove from essence TIP-30 Milestone Payload with Keys removed from essence Mar 25, 2022
@Wollac
Copy link
Contributor Author

Wollac commented Mar 25, 2022

Seems a bit strange it introduce a new TIP for this small change as you lose the option to view the diff. But I guess this is consistent with our process.

@lzpap lzpap changed the title TIP-30 Milestone Payload with Keys removed from essence TIP-29 Milestone Payload with Keys removed from essence Mar 25, 2022
Copy link
Member

@lzpap lzpap left a comment

Choose a reason for hiding this comment

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

Looks good to be merged as Draft.

@Wollac Wollac changed the title TIP-29 Milestone Payload with Keys removed from essence TIP-29 Milestone Payload (Stardust) Jul 7, 2022
Copy link
Member

@luca-moser luca-moser left a comment

Choose a reason for hiding this comment

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

The TIP does not outline the rule that a milestone's encompassing block must have a zero nonce.

tips/TIP-0029/tip-0029.md Outdated Show resolved Hide resolved
@Wollac
Copy link
Contributor Author

Wollac commented Jul 19, 2022

The TIP does not outline the rule that a milestone's encompassing block must have a zero nonce.

Where were you thinking of putting it in the tip, @luca-moser ?

I am honestly unsure whether it really belong in this TIP. The nonce is part of the block and thus also the pow validation is part of the block validation and not part of the syntactic validation of a milestone. We could of course add a remark in the Motivation or Rational section also explaining why this approach was taken, but I am not sure whether this would clarify things over what we already have in #55

@luca-moser
Copy link
Member

Alright, then lets not add it to this TIP. Makes sense that one would have to read the block TIP anyway in order to correctly be able to construct a block with a milestone in it. @Wollac

tips/TIP-0029/tip-0029.md Outdated Show resolved Hide resolved
tips/TIP-0029/tip-0029.md Show resolved Hide resolved
To increase the security of the design, a milestone can (optionally) be independently signed by multiple keys at once. These keys should be operated by detached signature provider services running on independent infrastructure elements. This assists in mitigating the risk of an attacker having access to all the key material necessary for forging milestones. While the Coordinator takes responsibility for forming Milestone Payload Blocks, it delegates signing in to these providers through an ad-hoc RPC connector. Mutual authentication should be enforced between the Coordinator and the signature providers: a [client-authenticated TLS handshake](https://en.wikipedia.org/wiki/Transport_Layer_Security#Client-authenticated_TLS_handshake) scheme is advisable. To increase the flexibility of the mechanism, nodes can be configured to require a quorum of valid signatures to consider a milestone as genuine.

In addition, a key rotation policy can also be enforced by limiting key validity to certain milestone intervals. Accordingly, nodes need to know which public keys are applicable for which milestone index. This can be provided by configuring a list of entries consisting of the following fields:
- _Index Range_ providing the interval of milestone indices for which this entry is valid. The interval must not overlap with any other entry.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not how it is implemented!
Key ranges are allowed to overlap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked the Hornet config and there it seems we have (@muXxer please correct if wrong):

  • A list of key ranges, each element with:
    • public key
    • (potentially overlapping) range
  • A global config for the threshold (which is then applied to all key ranges)

Technically, it is equivalent to either have an overlapping list of single keys or a non-overlapping list of sets of key (like in the TIP).
@muXxer, I suppose you are suggesting to change the TIP to reflect the configuration format which is already implemented in Hornet?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I misunderstood that part. Thats even better than it is implemented right now in hornet, because you can specify the threshold per range. And it's easier to read.

So the TIP is fine, maybe we should change the format in hornet in an upcoming version.

Copy link
Contributor

Choose a reason for hiding this comment

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

tips/TIP-0029/tip-0029.md Outdated Show resolved Hide resolved
tips/TIP-0029/tip-0029.md Outdated Show resolved Hide resolved
lzpap and others added 3 commits July 21, 2022 17:36
Co-authored-by: muXxer <mux3r@web.de>
Co-authored-by: muXxer <mux3r@web.de>
@lzpap lzpap merged commit 1e5f1d5 into main Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants