Skip to content

Conversation

@Kubuxu
Copy link
Contributor

@Kubuxu Kubuxu commented Jun 7, 2024

No description provided.

@Kubuxu Kubuxu force-pushed the feat/save-certs branch from 7092190 to 81b8959 Compare June 7, 2024 14:34
@Kubuxu Kubuxu requested review from Stebalien and masih June 7, 2024 14:34
@Kubuxu Kubuxu force-pushed the feat/save-certs branch from 81b8959 to eb99b7d Compare June 7, 2024 14:35
@Kubuxu
Copy link
Contributor Author

Kubuxu commented Jun 7, 2024

Based on #320

@codecov
Copy link

codecov bot commented Jun 7, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.92%. Comparing base (c832456) to head (35c6279).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
+ Coverage   82.88%   82.92%   +0.04%     
==========================================
  Files          15       15              
  Lines        1683     1687       +4     
==========================================
+ Hits         1395     1399       +4     
  Misses        168      168              
  Partials      120      120              
Files Coverage Δ
gpbft/participant.go 84.79% <88.88%> (+0.36%) ⬆️

if err != nil {
return xerrors.Errorf("forming certificate out of decision: %w", err)
}
err = h.client.certstore.Put(h.runningCtx, cert)
Copy link
Member

Choose a reason for hiding this comment

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

IMO, we should spend the extra time and validate this certificate. We can either do that inside the certstore, or here.

@Kubuxu Kubuxu force-pushed the feat/save-certs branch from eb99b7d to 29bc385 Compare June 12, 2024 15:08
Signed-off-by: Jakub Sztandera <oss@kubuxu.com>
@Kubuxu Kubuxu force-pushed the feat/save-certs branch from 29bc385 to 35c6279 Compare June 12, 2024 15:08
if err != nil {
return xerrors.Errorf("forming certificate out of decision: %w", err)
}
_, _, _, err = certs.ValidateFinalityCertificates(h, h.NetworkName(), current.Entries, decision.Vote.Instance, cert.ECChain.Base())
Copy link
Member

Choose a reason for hiding this comment

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

note: you don't need to pass the base. If you pass nil, it'll just ignore it.

@Kubuxu Kubuxu added this pull request to the merge queue Jun 12, 2024
Merged via the queue into main with commit c72726a Jun 12, 2024
@Kubuxu Kubuxu deleted the feat/save-certs branch June 12, 2024 15:53
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.

3 participants