Introduce k parameter #1827
Introduce k parameter #1827
Conversation
|
bors try |
tryBuild succeeded |
| protocolParametersFeePolicy W.FeePolicy sql=fee_policy | ||
| protocolParametersTxMaxSize Word16 sql=tx_max_size | ||
| protocolParametersDecentralizationLevel Percentage sql=decentralization_level | ||
| protocolParametersDesiredNumberOfPools Word16 Maybe sql=desired_pool_number |
There was a problem hiding this comment.
I looked at PParams in ledger for byron - there is no such a thing. So it does not make sense to talk about it (of course there is no staking for byron). So I thought Nothing there make sense. For jormungandr there is no such thing exposed so also set Nothing (although it is sensible as stake pools are there). For shelley we have this parameter in ledger nOpt and we can query the desired number of stake pool (which is 100 btw)
There was a problem hiding this comment.
There's pool_participation_capping for Jörmungandr, which has the same meaning. For Byron, we could default to 0 here, I think it'll be less misleading than having to carry a Maybe around for Shelley :/
There was a problem hiding this comment.
ok, will use Word16 instead of Maybe Word16 with 0 for Byron
There was a problem hiding this comment.
done in 3957dbc (also used pool participation capping max value for jormunagndr)
paweljakubas
force-pushed
the
paweljakubas/1822/introduce-k-parameter
branch
from
d231dfb to
c3d24aa
Compare
|
bors try |
tryBuild failed |
KtorZ
added
the
ADDING FEATURE
|
error not related to this PR to my best knowledge: |
|
@paweljakubas yes - that test failure is due to #1831. |
… present for shelley
paweljakubas
force-pushed
the
paweljakubas/1822/introduce-k-parameter
branch
from
c3d24aa to
af7fc9c
Compare
| @@ -1409,6 +1413,7 @@ instance Buildable ProtocolParameters where | |||
| build pp = blockListF' "" id | |||
| [ "Decentralization level: " <> build (pp ^. #decentralizationLevel) | |||
| , "Transaction parameters: " <> build (pp ^. #txParameters) | |||
| , "K parameter: " <> build (pp ^. #desiredNumberOfStakePools) | |||
There was a problem hiding this comment.
I'd actually go for "desired number of pools" here too. A bit clearer since there are at least 3 things called k in the shelley specs ^^"
There was a problem hiding this comment.
done in 499d4eb
|
bors r+ |
1827: Introduce k parameter r=paweljakubas a=paweljakubas # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #1822 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have extended ApiNetworkParameters for desired number of pools. This is optional as it does not make sense for byron, it is unknown in jormungandr - [x] I have made the changes in swagger - [x] I have updated unit tests in core, especially made a change to Arbitrary for ApiNetworkParameters to allow Word16. - [x] I have added 0 to byron and used max value of participation capping in jormungandr - [x] I have added impl in shelley using ledger accessors - [x] I have updated all other places in the code to account for this change - [x] I have extended integration network test for shelley and see that desired number of pools is 3. # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Pawel Jakubas <[email protected]>
Build failed |
|
bors retry |
Build succeeded |
Issue Number
#1822
Overview
Comments