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.

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

Network Spec Update #5036

Merged
merged 26 commits into from
Jan 16, 2025
Merged

Network Spec Update #5036

merged 26 commits into from
Jan 16, 2025

Conversation

coot
Copy link
Contributor

@coot coot commented Jan 13, 2025 •
edited
Loading

Description

Fixes:

Checklist

Quality

  • Commit sequence makes sense and have useful messages, see ref.
  • New tests are added and existing tests are updated.
  • Self-reviewed the PR.

Maintenance

  • Linked an issue or added the PR to the current sprint of ouroboros-network project.
  • Added labels.
  • Updated changelog files.
  • The documentation has been properly updated, see ref.

@coot coot requested a review from a team as a code owner January 13, 2025 12:57
This was linked to issues Jan 13, 2025
Document Local Tx-Monitor mini-protocol #3420
Closed
Links to implemented mini-protocols are obsolete #2780
Closed
@coot coot self-assigned this Jan 14, 2025
@coot coot added the documentation Network Documentation related tasks label Jan 14, 2025
@coot
Copy link
Contributor Author

coot commented Jan 14, 2025

Here's the update document:
network-spec.pdf

karknu
karknu approved these changes Jan 14, 2025
View reviewed changes
Copy link
Contributor

@karknu karknu left a comment

Choose a reason for hiding this comment

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

LGTM.

Can you please update my email while you're changing the file?

ouroboros-network-protocols/CHANGELOG.md Outdated
* Added kind signatures to protocol types:
* `ChainSync`,
* `BlockFetch`,
* `KeepAlive`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I can not find the kind signature update for the KeepAlive protocol. Did you forget to commit it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KeepAlive doesn't have any polymorphic parameters, so there's no need.

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 updated the CHANGELOG file: it should mention TxSubmission2 and not KeepAlive.

@coot coot force-pushed the coot/network-spec-update branch 2 times, most recently from 38326a5 to c1114c3 Compare January 14, 2025 11:00
bolt12
bolt12 previously requested changes Jan 14, 2025
View reviewed changes
Copy link
Contributor

@bolt12 bolt12 left a comment

Choose a reason for hiding this comment

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

LGTM! I did notice the same thing as @karknu did, perhaps you want to change that

@coot coot force-pushed the coot/network-spec-update branch 3 times, most recently from aa476c1 to b04b03c Compare January 14, 2025 16:56
@coot coot enabled auto-merge January 14, 2025 16:57
@coot coot requested a review from bolt12 January 14, 2025 16:57
coot added 14 commits January 14, 2025 18:16
@coot
ouroboros-network-protocols: added kind signatures
c31e53b 
This improves haddocks.
@coot
network-spec: fixed links
3bb050a
@coot
network-spec: moved peer-sharing mini-protocol section
b39d2c3 
Let's keep all node-to-node protocols together.
@coot
network-spec: organise mini-protocols links into subsections
3ea77f4
@coot
network-spec: added local tx-monitor mini-protocol description
aad6508
@coot
network-spec: various small edits
9d424f2
@coot
network-spec: added mini-protocol numbers
538db20
@coot
network-spec: fixed MsgRollBackward payload
3a675a7 
fixes: #4686
@coot
network-spec: aligned table
3ebfed4
@coot
network-spec: reference node-to-{client,node} protocol numbers
3c1ddfb
@coot
network-spec: added CDDL spec of Local Tx-Submission mini-protocol
6d3cce1 
Fixes: #4521
@coot
network-spec: fixed colors in Local Tx-Submission diagram
d2afa10 
Fixes: #4662
@coot
network-spec: specify meaning of the range
4e7e930 
Fixes: #4628
@coot
network-spec: documented protocol size limits and timeouts
28cb5e4
@coot coot force-pushed the coot/network-spec-update branch from b04b03c to 33e06e2 Compare January 14, 2025 17:16
@coot coot dismissed bolt12’s stale review January 14, 2025 18:55

changes were addressed

@coot coot added this pull request to the merge queue Jan 14, 2025
@coot coot removed this pull request from the merge queue due to a manual request Jan 14, 2025
@coot coot force-pushed the coot/network-spec-update branch from ef463b2 to 92577d3 Compare January 16, 2025 09:34
@coot coot enabled auto-merge January 16, 2025 09:34
@coot coot added this pull request to the merge queue Jan 16, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 16, 2025
@coot coot added this pull request to the merge queue Jan 16, 2025
Merged via the queue into main with commit 091260b Jan 16, 2025
12 checks passed
@coot coot deleted the coot/network-spec-update branch January 16, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@karknu karknu karknu approved these changes

@bolt12 bolt12 Awaiting requested review from bolt12

Assignees

@coot coot

Labels
documentation Network Documentation related tasks
Projects
Ouroboros Network
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Document Local Tx-Monitor mini-protocol Links to implemented mini-protocols are obsolete
3 participants
@coot @karknu @bolt12