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

Configurable protocol versions #830

Closed
wants to merge 5 commits into from
Closed

Configurable protocol versions #830

wants to merge 5 commits into from

Conversation

coot
Copy link
Contributor

@coot coot commented Apr 22, 2020 •
edited
Loading

Issue

Checklist

  • This PR contains all the work required to resolve the linked issue.

  • The work contained has sufficient documentation to describe what it does and how to do it.

  • The work has sufficient tests and/or testing (in ouroboros-network).

  • I have committed clear and descriptive commits. Be considerate as somebody else will have to read these.

  • I have added the appropriate labels to this PR.

@coot coot added the networking Issues and PRs related to networking label Apr 22, 2020
@coot coot requested review from dcoutts and Jimbo4350 April 22, 2020 05:32
@coot
Copy link
Contributor Author

coot commented Apr 22, 2020

@mrBliss if you don't mind 'aeson' dependency in ouroboros-consensus (and ouroboros-consensus-byron) I can move the orphaned FromJSON instances for NodeToNodeVersion and NodeToClientVersion to ouroboros-consensus-byron.

@mrBliss
Copy link
Contributor

mrBliss commented Apr 22, 2020

@mrBliss if you don't mind 'aeson' dependency in ouroboros-consensus (and ouroboros-consensus-byron) I can move the orphaned FromJSON instances for NodeToNodeVersion and NodeToClientVersion to ouroboros-consensus-byron.

I'm not keen on doing this. Do they have to be instances here? Doesn't aeson provide a way to use explicit parsers (in combination which parseJSONList). Moreover, aren't there tons of orphans already in cardano-node, e.g., for tracing?

@coot
Copy link
Contributor Author

coot commented Apr 22, 2020 •
edited
Loading

I thought so :);

Doesn't aeson provide a way to use explicit parsers (in combination which parseJSONList).

It seems so - the FromJSON instance has a default implementation for parseJSONList.

coot added 5 commits April 22, 2020 15:43
@coot
Added ByronNodeToClient and ByronNodeToNode configuration options
06d7328 
Both are lists of supported 'NodeToClientVersion' and
'NodeToNodeVersion' respectively (defined in
'Ouoroboros.Consensus.Node.NetworkProtocolVersion').
The default values are all supported protocols as provided by
'HasNetworkProtocolVersion' class instanct.
@coot
make advange of the new config options in applications
7893c55
@coot
Advertise the new options in configuration.yaml file
ac19eb6 
They are commented out, since the default (use all supported versions)
is the right choice now.
@coot
Updated cabal.project and stack.yaml files
a833156
@coot
Updated cardano-api
67a2aa1 
Removed NFData instances, since they are not needed anymore, and one
cannont derive them.
@coot coot force-pushed the coot/protocol-versions branch from 266d29a to 67a2aa1 Compare April 22, 2020 13:58
@coot coot mentioned this pull request Apr 24, 2020
Merged
@coot
Copy link
Contributor Author

coot commented May 27, 2020

stale PR, closing

@coot coot closed this May 27, 2020
@coot coot reopened this Jun 23, 2020
@coot
Copy link
Contributor Author

coot commented Jun 23, 2020 •
edited
Loading

Re-opened - at some point this will be updated.

@Jimbo4350
Copy link
Contributor

Spoke with @coot who agreed to close this

@Jimbo4350 Jimbo4350 closed this Dec 18, 2020
@erikd erikd deleted the coot/protocol-versions branch August 18, 2022 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcoutts dcoutts Awaiting requested review from dcoutts

@Jimbo4350 Jimbo4350 Awaiting requested review from Jimbo4350

@erikd erikd Awaiting requested review from erikd

@intricate intricate Awaiting requested review from intricate

@mrBliss mrBliss Awaiting requested review from mrBliss

@newhoggy newhoggy Awaiting requested review from newhoggy

Assignees
No one assigned
Labels
networking Issues and PRs related to networking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deserialise failure when connecting to FC4 node
3 participants
@coot @mrBliss @Jimbo4350