-
Notifications
You must be signed in to change notification settings - Fork 23
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
Use ShelleyNTC for version check in node queries #403
Use ShelleyNTC for version check in node queries #403
Conversation
91e1197
to
60f5254
Compare
60f5254
to
ec14f18
Compare
Net.Query.ClientStQuerying | ||
{ Net.Query.recvMsgResult = f | ||
} | ||
| otherwise -> pure . Left $ UnsupportedNtcVersionError minNtcVersion ntcVersion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- TODO: include shelley NTC version info and/or use separate error for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't look at this yet but I'm prematurely requesting changes as this needs to be carefully considered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new logic here looks correct to me.
We also talked about this in the Consensus Office Hours today, and came to the conclusion that we can avoid cardano-api
having to implement this more complicated logic by a simple change in Consensus, see IntersectMBO/ouroboros-consensus#862 and IntersectMBO/ouroboros-consensus#863.
Thanks @amesgen ! I think I prefer to rely on the consensus change and your assumption:
this way we can just use |
Closing because of: #403 (comment) |
Fix delegating to always-no-confidence
Changelog
Context
Fixes #391
How to trust this PR
Highlight important bits of the PR that will make the review faster. If there are commands the reviewer can run to observe the new behavior, describe them.
Checklist