Skip to content

feat: CON-1670 Add some DKG metrics & avoid proposing empty IDKG payloads#10121

Merged
eichhorl merged 159 commits into
masterfrom
eichhorl/dkg-metrics
May 12, 2026
Merged

feat: CON-1670 Add some DKG metrics & avoid proposing empty IDKG payloads#10121
eichhorl merged 159 commits into
masterfrom
eichhorl/dkg-metrics

Conversation

@eichhorl
Copy link
Copy Markdown
Contributor

@eichhorl eichhorl commented May 7, 2026

Add metrics for

  • NiDKG payload/transcript creation errors
  • NiDKG payload creation time
  • Number of dealings & remote transcripts included in data payloads
  • Sum and size of DKG attempts map in summary payloads

Additionally, we change the way IDKG payload creation errors are handled: Previously, if no IDKG payload could be created, the block maker would instead propose a block without an IDKG payload. This is dangerous since we could silently loose the master keys on the subnet, even if the reason for the payload creation failure was potentially transient.

After this PR, if IDKG payload creation fails, we will not propose a block. Instead we will try again later, or wait until we receive a block from different block maker.

@github-actions github-actions Bot added the chore label May 7, 2026
Base automatically changed from eichhorl/early-dkg-configs to master May 12, 2026 09:43
@eichhorl eichhorl changed the title chore: Add some DKG metrics feat: CON-1670 Add some DKG metrics & avoid proposing empty IDKG payloads May 12, 2026
@github-actions github-actions Bot added feat and removed chore labels May 12, 2026
@eichhorl eichhorl marked this pull request as ready for review May 12, 2026 10:53
@eichhorl eichhorl requested a review from a team as a code owner May 12, 2026 10:54
@eichhorl eichhorl requested a review from pierugo-dfinity May 12, 2026 10:54
Copy link
Copy Markdown
Contributor

@pierugo-dfinity pierugo-dfinity left a comment

Choose a reason for hiding this comment

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

This is dangerous since we could silently loose the master keys on the subnet

AFAICT, others would not validate the block as they would either compute the actual summary or also hit an error no? I still agree that it makes more sense to not propose the block in case of an error though.

Comment thread rs/consensus/dkg/src/metrics.rs Outdated
@eichhorl
Copy link
Copy Markdown
Contributor Author

This is dangerous since we could silently loose the master keys on the subnet

AFAICT, others would not validate the block as they would either compute the actual summary or also hit an error no? I still agree that it makes more sense to not propose the block in case of an error though.

Good point, and data blocks would be rejected with InvalidIDkgPayloadReason::MissingIDkgDataPayload as well 👍

@eichhorl eichhorl enabled auto-merge May 12, 2026 13:55
@eichhorl eichhorl added this pull request to the merge queue May 12, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 12, 2026
@eichhorl eichhorl added this pull request to the merge queue May 12, 2026
Merged via the queue into master with commit 74b5672 May 12, 2026
63 of 64 checks passed
@eichhorl eichhorl deleted the eichhorl/dkg-metrics branch May 12, 2026 16:36
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.

3 participants