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

Pass ControlMessageSTM to keep-alive client #4168

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

coot
Copy link
Contributor

@coot coot commented Nov 21, 2022

Description

Fixes #4163.

Checklist

  • Branch
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • New tests are added if needed and existing tests are updated
    • If this branch changes Consensus and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@coot coot added keep-alive Issues / PRs related to keep-alive protocol networking labels Nov 21, 2022
@coot coot requested a review from bolt12 November 21, 2022 09:10
@coot
Copy link
Contributor Author

coot commented Nov 21, 2022

BTW, this fix does not introduces a bug because each protocol temperature is supported by a separate TVar from which we read the ControlMessage, ref.

@coot coot mentioned this pull request Nov 21, 2022
9 tasks
@coot
Copy link
Contributor Author

coot commented Nov 21, 2022

bors merge

iohk-bors bot added a commit that referenced this pull request Nov 21, 2022
4144: Add support for CHaP and ghc-9.2 r=angerman a=erikd

Builds with both ghc-8.10 and ghc-9.2.

Currently uses a `source-package-repository` reference to a `cardano-ledger` PR that has not yet been merged.

4168: Pass ControlMessageSTM to keep-alive client r=coot a=coot

# Description

Fixes #4163.



Co-authored-by: Erik de Castro Lopo <erikd@mega-nerd.com>
Co-authored-by: Marcin Szamotulski <coot@coot.me>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 21, 2022

Build failed (retrying...):

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 21, 2022

@iohk-bors iohk-bors bot merged commit 567f9ba into master Nov 21, 2022
@iohk-bors iohk-bors bot deleted the coot/fix-keep-alive-control-msg branch November 21, 2022 14:28
iohk-bors bot added a commit that referenced this pull request Nov 21, 2022
4170: Pick two changes for 1.35.5 r=coot a=coot

# Description

Pick two commits from the following two PRs:

* #4167
* #4168
* #4159 (to fix GitHub Actions on Windows)

**Note:** if we decide two include these patches in `1.35.5`, they also must bee included in `1.36.0`.

# Checklist

- Branch
    - [x] Commit sequence broadly makes sense
    - [x] Commits have useful messages
    - [ ] New tests are added if needed and existing tests are updated
    - [ ] If this branch changes Consensus and has any consequences for downstream repositories or end users, said changes must be documented in [`interface-CHANGELOG.md`](../ouroboros-consensus/docs/interface-CHANGELOG.md)
    - [ ] If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in [`interface-CHANGELOG.md`](../docs/interface-CHANGELOG.md)
    - [ ] If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
- Pull Request
    - [x] Self-reviewed the diff
    - [ ] Useful pull request description at least containing the following information:
      - What does this PR change?
      - Why these changes were needed?
      - How does this affect downstream repositories and/or end-users?
      - Which ticket does this PR close (if any)? If it does, is it [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)?
    - [x] Reviewer requested


Co-authored-by: Marcin Szamotulski <coot@coot.me>
Co-authored-by: Karl Knutsson <karl.fb.knutsson@gmail.com>
Co-authored-by: Moritz Angermann <moritz.angermann@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep-alive Issues / PRs related to keep-alive protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keep alive will not stop when instrumented to do so
3 participants