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.

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

Typed Protocols: new API #4935

Merged
merged 33 commits into from
Sep 27, 2024
Merged

Typed Protocols: new API #4935

merged 33 commits into from
Sep 27, 2024

Conversation

coot
Copy link
Contributor

@coot coot commented Aug 21, 2024

Description

Using input-output-hk/typed-protocols#52 & input-output-hk/typed-protocols#61.

TODO:

Performance

No performance regression was found when running the benchmarking cluster using
cardano-node-9.1.0 as a base. No regression was found when syncing mainnet.

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 mentioned this pull request Aug 21, 2024
8 tasks
@coot coot changed the title Typed Protocols - new API Typed Protocols: new API Aug 21, 2024
@coot coot force-pushed the coot/typed-protocols-new-api branch 3 times, most recently from 4b8c1f7 to f270fd5 Compare August 22, 2024 13:14
@coot coot added the typed-protocols Issues related to typed-protocols label Aug 22, 2024
@coot coot force-pushed the coot/typed-protocols-new-api branch from f270fd5 to 4242d6d Compare August 22, 2024 16:30
@coot coot marked this pull request as ready for review August 23, 2024 06:02
@coot coot requested review from newhoggy and a team as code owners August 23, 2024 06:02
@coot coot force-pushed the coot/typed-protocols-new-api branch from 4242d6d to d295ced Compare September 16, 2024 19:03
@coot coot self-assigned this Sep 16, 2024
@crocodile-dentist
Copy link
Contributor

Not related to the PR directly but I guess we could sneak it here, there's a usage binding in the module chain-sync that is not referenced anywhere - maybe we can remove it completely?

@coot coot force-pushed the coot/typed-protocols-new-api branch 3 times, most recently from f2cf734 to 2b81820 Compare September 20, 2024 12:09
@coot
Copy link
Contributor Author

coot commented Sep 23, 2024

Not related to the PR directly but I guess we could sneak it here, there's a usage binding in the module chain-sync that is not referenced anywhere - maybe we can remove it completely?

Which one?

@coot coot force-pushed the coot/typed-protocols-new-api branch from 2b81820 to 234f3fc Compare September 23, 2024 14:20
@crocodile-dentist
Copy link
Contributor

Not related to the PR directly but I guess we could sneak it here, there's a usage binding in the module chain-sync that is not referenced anywhere - maybe we can remove it completely?

Which one?

It's this one: https://github.com/IntersectMBO/ouroboros-network/blob/coot/tx-submission/ouroboros-network/demo/chain-sync.hs#L191-L194

@coot coot force-pushed the coot/typed-protocols-new-api branch 3 times, most recently from 8f7be2d to 7d73076 Compare September 26, 2024 10:22
ouroboros-network/demo/chain-sync.hs Outdated Show resolved Hide resolved
ouroboros-network/CHANGELOG.md Outdated Show resolved Hide resolved
ouroboros-network-api/CHANGELOG.md Outdated Show resolved Hide resolved
ouroboros-network-framework/CHANGELOG.md Outdated Show resolved Hide resolved
ouroboros-network-protocols/CHANGELOG.md Outdated Show resolved Hide resolved
@coot coot force-pushed the coot/typed-protocols-new-api branch from 7d73076 to babfabf Compare September 26, 2024 12:35
Copy link
Contributor

@crocodile-dentist crocodile-dentist left a comment

Choose a reason for hiding this comment

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

LGTM

@coot coot force-pushed the coot/typed-protocols-new-api branch 2 times, most recently from 1f7109a to 240cf23 Compare September 26, 2024 15:38
Most part of this patch is updating dependencies.
This is only useful for debugging a failing test case.
We use pipelined client, hence we need to use an buffered channel.
The client side of each block-fetch protocol has to run until it's
completion, otherwise it might happen that we will not generate a trace
which marks completion of fetching all blocks in a batch
(`CompletedFetchBatch`).  This results in failure of
`tracePropertyInFlight`.
The first assertion of `fetchDecisionsForStateSnapshot` was triggered.
We need first kill the fetch thread before the client side terminates.
This is useful API for running stateful peers.
ProtocolTimeLimits can be a newtype.
Allow to build against nothunks-0.1; `ouroboros-consensus` and
`plutus-core` do not yet use `nothunks-0.2`.
This ought to be part of the PR which introduced `NumTxIdsToAck`.
typed-protocols-0.3.0.0 provide better stateful API, which allows us to
decouple `Message` type from it encoding.  This is used to remove `query
resesult` field of `MsgResp` of the `LocalStateQuery` mini-protocol.
@coot coot force-pushed the coot/typed-protocols-new-api branch from 0adc34b to e66922b Compare September 27, 2024 18:29
@coot coot enabled auto-merge September 27, 2024 18:29
@coot coot added this pull request to the merge queue Sep 27, 2024
Merged via the queue into master with commit faf4c69 Sep 27, 2024
23 checks passed
@coot coot deleted the coot/typed-protocols-new-api branch September 27, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typed-protocols Issues related to typed-protocols
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants