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

Extension of PoolCurrentPaydayInfo/PoolInfoResponse #587

Merged
merged 4 commits into from
Jan 8, 2025
Merged

Conversation

drsk0
Copy link
Contributor

@drsk0 drsk0 commented Dec 19, 2024

Purpose

Add suspension info to PoolCurrentPaydayInfo / PoolInfoResponse. This depends on Concordium/concordium-grpc-api#74.

Changes

See above.

Checklist

  • My code follows the style of this project.
  • The code compiles without warnings.
  • I have performed a self-review of the changes.
  • I have documented my code, in particular the intent of the
    hard-to-understand areas.
  • (If necessary) I have updated the CHANGELOG.

CLA acceptance

_Remove if not applicable.

By submitting the contribution I accept the terms and conditions of the
Contributor License Agreement v1.0

@drsk0 drsk0 force-pushed the pool_info_response branch 3 times, most recently from b467dcb to 7ddf501 Compare January 7, 2025 09:47
@drsk0 drsk0 force-pushed the pool_info_response branch from 7ddf501 to 1e05bb3 Compare January 7, 2025 09:50
Copy link
Contributor

@td202 td202 left a comment

Choose a reason for hiding this comment

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

It might be worth making a explicit ToJSON/FromJSON instances for CurrentPaydayBakerPoolStatus to be consistent in how Nothing values are represented. Currently, isPrimedForSuspension and missedRounds are represented as null values if Nothing, while isSuspended is just not present if it's Nothing.

Current output of GetPoolStatus:

{
    "allPoolTotalCapital": "57662194352796877",
    "bakerAddress": "48XGRnvQoG92T1AwETvW5pnJ1aRSPMKsWtGdKhTqyiNZzMk3Qn",
    "bakerEquityCapital": "6621352845013062",
    "bakerId": 0,
    "bakerStakePendingChange": {
        "pendingChangeType": "NoChange"
    },
    "currentPaydayStatus": {
        "bakerEquityCapital": "6620269533524289",
        "blocksBaked": 2740,
        "commissionRates": {
            "bakingCommission": 0.1,
            "finalizationCommission": 1.0,
            "transactionCommission": 0.1
        },
        "delegatedCapital": "0",
        "effectiveStake": "2882948611945322",
        "finalizationLive": true,
        "isPrimedForSuspension": null,
        "lotteryPower": 9.978157861545638e-2,
        "missedRounds": null,
        "transactionFeesEarned": "188108074"
    },
    "delegatedCapital": "0",
    "delegatedCapitalCap": "0",
    "poolInfo": {
        "commissionRates": {
            "bakingCommission": 0.1,
            "finalizationCommission": 1.0,
            "transactionCommission": 0.1
        },
        "metadataUrl": "",
        "openStatus": "closedForAll"
    },
    "poolType": "BakerPool"
}

@drsk0
Copy link
Contributor Author

drsk0 commented Jan 8, 2025

@td202 So we would also change how other optional values are represented in BakerPoolStatus, like activeStatusFields?

@td202
Copy link
Contributor

td202 commented Jan 8, 2025

@td202 So we would also change how other optional values are represented in BakerPoolStatus, like activeStatusFields?

No, I don't think we should change BakerPoolStatus. I just mean that instead of isPrimedForSuspension and missedRounds being null if they're Nothing, they should just not be present in the JSON object (as is the case in BakerPoolStatus).

@drsk0 drsk0 force-pushed the pool_info_response branch from b7319b3 to a5dd91e Compare January 8, 2025 17:42
@drsk0 drsk0 force-pushed the pool_info_response branch from a5dd91e to e662e70 Compare January 8, 2025 17:42
@drsk0
Copy link
Contributor Author

drsk0 commented Jan 8, 2025

I see, I added omitNothingFields = True in the defaultOptions of the deriveJSON slice for CurrentPaydayBakerPoolStatus, this will have the desired effect.

@drsk0 drsk0 merged commit 92150ab into main Jan 8, 2025
40 checks passed
@drsk0 drsk0 deleted the pool_info_response branch January 8, 2025 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants