-
Notifications
You must be signed in to change notification settings - Fork 3
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
JAV-319 Support for Protocol 7 #341
Conversation
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.
IMO looks good. You need to handle the newly added transaction events however.
Have a look at the added transaction events here: BakerEvent.DelegationRemoved and DelegationEvent.BakerRemoved
concordium-sdk/src/main/java/com/concordium/sdk/responses/poolstatus/BakerPoolStatus.java
Show resolved
Hide resolved
It seems that the SDK doesn't handle any events, they are only found in gRPC generated classes. |
I don't understand why I cannot find the added types anywhere then? I guess the generated code is not part of the repository? To me it looks like anything in https://github.com/Concordium/concordium-java-sdk/tree/JAV-319-protocol-7/concordium-sdk/src/main/java/com/concordium/sdk/responses contains classes which are created from the generated GRPC types, and this folder https://github.com/Concordium/concordium-java-sdk/tree/JAV-319-protocol-7/concordium-sdk/src/main/java/com/concordium/sdk/responses/transactionstatus is supposed to (I assume) have 2 extra classes for the respective events linked in my original review:
And of course these would have to be mapped as well somewhere.. If the events are not used it seems like a lot of wasted effort was put into making the already existing conversions? |
Breaking changes in BakerPoolStatus
Also remove bakerId from AbstractBakerResult.
Also remove delegatorId from AbstractDelegatorResult.
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.
Some minor inconsistencies. When they're fixed, feel free to merge 😄
Purpose
Support Protocol 7.
Changes
ProtocolVersion.V7
corresponding to Protocol version 7cooldowns
list toAccountInfo
availableBalance
toAccountInfo
BakerPoolStatus
fields:bakerEquityCapital
,delegatedCapital
,delegatedCapitalCap
,poolInfo
7.3.0-SNAPSHOT
Checklist
hard-to-understand areas.