Skip to content

[WIP] Implement and wire-in the ObjectDiffusion miniprotocol for Peras certificate diffusion #1615

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 7 commits into
base: peras-staging
Choose a base branch
from
Draft

[WIP] Implement and wire-in the ObjectDiffusion miniprotocol for Peras certificate diffusion #1615

wants to merge 7 commits into from
+1,487 −19

Conversation

tbagrel1
Copy link

@amesgen amesgen added the Peras label Jul 31, 2025
@tbagrel1 tbagrel1 force-pushed the peras/object-diffusion branch from c132caf to 282c91a Compare August 6, 2025 17:00
@tbagrel1 tbagrel1 requested review from amesgen and Niols August 6, 2025 17:01
@tbagrel1
Copy link
Author

tbagrel1 commented Aug 6, 2025

Alright, I cleaned up most of the code, and separated it into 5 (hopefully) digestible commits. Ready for review!

This was linked to issues Aug 6, 2025
Get a basic object diffusion working tweag/cardano-peras#57
Open
Design how to get certificates from PerasCertDB for diffusion tweag/cardano-peras#77
Open
@tbagrel1
Copy link
Author

tbagrel1 commented Aug 6, 2025 •
edited
Loading

Important note: the definition of ObjectDiffusion in ouroboros-network relies on the InboundAgency and OutboundAgency types (see ouroboros-network-protocols/src/Ouroboros/Network/Protocol/ObjectDiffusion/Type.hs). So far, they are aliases for hardcoded ServerAgency and ClientAgency. It doesn't bother us too much for now, as client/server roles are purely arbitrary and symmetrical from a typed-protocols PoV, but apparently for production they carry a particular meaning, so we should find a way to fix that.

@tbagrel1 tbagrel1 mentioned this pull request Aug 7, 2025
Draft
amesgen
amesgen reviewed Aug 7, 2025
View reviewed changes
Copy link
Member

@amesgen amesgen 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, left a few comments/suggestions

@amesgen amesgen mentioned this pull request Aug 7, 2025
Open
@amesgen amesgen force-pushed the peras/object-diffusion branch from 282c91a to d9ceb01 Compare August 13, 2025 15:51
@tbagrel1 tbagrel1 force-pushed the peras/object-diffusion branch from dace000 to 0e77b7b Compare August 21, 2025 08:42
@tbagrel1 tbagrel1 requested a review from amesgen August 21, 2025 08:48
amesgen
amesgen approved these changes Aug 22, 2025
View reviewed changes
Copy link
Member

@amesgen amesgen left a comment

Choose a reason for hiding this comment

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

LGTM, only minor comments

@amesgen
Copy link
Member

amesgen commented Aug 22, 2025

Also, there are some CI failures.

  • The x86_64-darwin and windows failures are flaky and can be ignored.
  • The fourmolu and hlint failures are legit.
  • The cabal-docspec failure can be fixed by rebasing on top of peras-staging.

Niols
Niols approved these changes Aug 25, 2025
View reviewed changes
Copy link
Contributor

@Niols Niols left a comment

Choose a reason for hiding this comment

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

This looks great! Buncha comments.

@amesgen amesgen mentioned this pull request Aug 27, 2025
Closed
Niols
Niols reviewed Aug 29, 2025
View reviewed changes
Copy link
Contributor

@Niols Niols left a comment

Choose a reason for hiding this comment

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

Made a few changes according to my own review. Feel free to revert them, obviously.

@amesgen amesgen force-pushed the peras/object-diffusion branch from 2524c4d to de3deab Compare August 29, 2025 17:22
@tbagrel1 tbagrel1 force-pushed the peras/object-diffusion branch from 8b0f277 to cd32fac Compare September 2, 2025 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Niols Niols Niols approved these changes

@amesgen amesgen amesgen approved these changes

@nfrisby nfrisby Awaiting requested review from nfrisby nfrisby will be requested when the pull request is marked ready for review nfrisby is a code owner

@jasagredo jasagredo Awaiting requested review from jasagredo jasagredo will be requested when the pull request is marked ready for review jasagredo is a code owner

@fraser-iohk fraser-iohk Awaiting requested review from fraser-iohk fraser-iohk will be requested when the pull request is marked ready for review fraser-iohk is a code owner

@dnadales dnadales Awaiting requested review from dnadales dnadales will be requested when the pull request is marked ready for review dnadales is a code owner

@geo2a geo2a Awaiting requested review from geo2a geo2a will be requested when the pull request is marked ready for review geo2a is a code owner

Assignees
No one assigned
Labels
Peras
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Design how to get certificates from PerasCertDB for diffusion Get a basic object diffusion working
3 participants
@tbagrel1 @amesgen @Niols