Skip to content
Snippets Groups Projects

Switch witness interaction to use the add-checkpoint request

Merged Niels Möller requested to merge nisse/witness-AddCheckpoint into main
1 unresolved thread

Merge request reports

Pipeline #5002 passed

Pipeline passed for 63041b3d on nisse/witness-AddCheckpoint

Merged by Niels MöllerNiels Möller 3 months ago (Sep 30, 2024 7:12am UTC)

Merge details

  • Changes merged into with 4a85762e.
  • Deleted the source branch.

Pipeline #5003 passed

Pipeline passed for 4a85762e on main

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Rasmus Dahlberg
  • Looks good! (I reviewed all of witness.go, because that was easier than seeing a diff for something I can't recall looking at. Then quickly looked at the diff and also didn't see anything that stood out then. I didn't review the test cases in detail, but a lot of green so I'm happy about that!)

  • Rasmus Dahlberg approved this merge request

    approved this merge request

  • Rasmus Dahlberg removed review request for @rgdd

    removed review request for @rgdd

  • 120 125
    121 126 cosignatures := make(map[crypto.Hash]types.Cosignature)
    122 127 for i := range ch {
    123 128 // TODO: Check that cosignature timestamp is reasonable?
    • (Btw: I think it would be worthwhile to address this in a separate MR. Especially as we start integrating with other witness implementations like AW. It would be nice to be first-on-the-ball and detect potential issues. Just a course-grained check would go a long way.)

    • Would make sense to me to warn if timestamp is more than one second into the future (indicates broken clock synchronization), or more than a few minutes old (since some delay is not particularly harmful, and I can imagine valid(?) configurations with considerable delay, e.g., a witness returning an old cached cosignature for some time if tree head is unchanged and making a fresh cosignature is expensive). But leaving for a later MR.

    • Please register or sign in to reply
  • Niels Möller added 1 commit

    added 1 commit

    • 63041b3d - Retry with new old size only once

    Compare with previous version

  • Niels Möller mentioned in commit 4a85762e

    mentioned in commit 4a85762e

  • Please register or sign in to reply
    Loading