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

chore: Update the default genesis params #285

Closed
wants to merge 7 commits into from
Closed

chore: Update the default genesis params #285

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6172672
chore: update the default mint params
evan-forbes Nov 16, 2022
938aa8d
chore: update the slashing modules default params
evan-forbes Nov 16, 2022
8e25e29
chore: update the default staking module params
evan-forbes Nov 16, 2022
94e00fd
chore: update the default params for the auth module
evan-forbes Nov 16, 2022
0215173
chore: add docs to default changes
evan-forbes Nov 16, 2022
0e05cb2
chore: use zero dec for inflation rate change
evan-forbes Nov 16, 2022
ea05762
chore: revert mint module default params changes
evan-forbes Nov 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions x/auth/types/params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

// Default parameter values
const (
DefaultMaxMemoCharacters uint64 = 256
DefaultMaxMemoCharacters uint64 = 128
Copy link
Member Author

Choose a reason for hiding this comment

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

this cuts the max memo in half. The memo is now 1/4 the size of a single share, and costs 25% more per gas (10 gas/byte vs 8 gas/ blob byte)

Copy link
Member

Choose a reason for hiding this comment

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

I think we can explore how the memo is currently used. And limit the number to the minimum necessary. also, In celestia people can exchange messages directly in the blobs instead of the memo field.

DefaultTxSigLimit uint64 = 7
DefaultTxSizeCostPerByte uint64 = 10
DefaultSigVerifyCostED25519 uint64 = 590
Expand Down Expand Up @@ -71,8 +71,10 @@ func DefaultParams() Params {

// SigVerifyCostSecp256r1 returns gas fee of secp256r1 signature verification.
// Set by benchmarking current implementation:
// BenchmarkSig/secp256k1 4334 277167 ns/op 4128 B/op 79 allocs/op
// BenchmarkSig/secp256r1 10000 108769 ns/op 1672 B/op 33 allocs/op
//
// BenchmarkSig/secp256k1 4334 277167 ns/op 4128 B/op 79 allocs/op
// BenchmarkSig/secp256r1 10000 108769 ns/op 1672 B/op 33 allocs/op
//
// Based on the results above secp256k1 is 2.7x is slwer. However we propose to discount it
// because we are we don't compare the cgo implementation of secp256k1, which is faster.
func (p Params) SigVerifyCostSecp256r1() uint64 {
Expand Down
8 changes: 4 additions & 4 deletions x/mint/types/params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ func NewParams(
func DefaultParams() Params {
return Params{
MintDenom: sdk.DefaultBondDenom,
InflationRateChange: sdk.NewDecWithPrec(13, 2),
InflationMax: sdk.NewDecWithPrec(20, 2),
InflationMin: sdk.NewDecWithPrec(7, 2),
InflationRateChange: sdk.ZeroDec(),
Copy link
Member

Choose a reason for hiding this comment

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

What does this do?

Copy link
Member Author

Choose a reason for hiding this comment

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

setting to zero doesn't allow for the inflation rate to change.

Copy link
Member

Choose a reason for hiding this comment

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

Note that this was set to 100% or 1 on the hub not too long ago. Zaki has some charts for rationale here.

InflationMax: sdk.NewDecWithPrec(2, 2),
InflationMin: sdk.NewDecWithPrec(2, 2),
Copy link
Member

Choose a reason for hiding this comment

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

We have not made any decision on the inflation parameters yet. It likely won't be 2% fixed though as that's probably too low.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, good to know.

Copy link
Member Author

Choose a reason for hiding this comment

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

this was initially changed because of our notes indicated heading in this direction (lower the percentage and keep it fixed)

Decided
we want a lower value for inflation
we want to set inflation max and inflation min to the same value
Undecided
the exact values for inflation are tbd

reverted here ea05762 and we can just rely on changing these params manually per network until we decide on values

Copy link
Member

Choose a reason for hiding this comment

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

We do want to reduce the values for inflation - but exact values are tbd as stated in notes

Copy link
Member

Choose a reason for hiding this comment

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

My thinking for inflation is a 3-5 % range (not flat). But we will decide this soonish.

GoalBonded: sdk.NewDecWithPrec(67, 2),
BlocksPerYear: uint64(60 * 60 * 8766 / 5), // assuming 5 second block times
BlocksPerYear: uint64(60 * 60 * 8766 / 15), // assuming 15 second block times
Copy link
Member

Choose a reason for hiding this comment

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

How is this used by the Cosmos SDK? Got a link to docs maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

blocks per year is used to determine the rate of change for inflation (we currently have the max inflation rate change set to 0, so this value shouldn't be important unless we ever change it)

https://docs.cosmos.network/main/modules/mint

}
}

Expand Down
2 changes: 1 addition & 1 deletion x/slashing/types/params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

// Default parameter namespace
const (
DefaultSignedBlocksWindow = int64(100)
DefaultSignedBlocksWindow = int64(5760) // 4 (blocks per min) * 60 (mins in hour) * 24 (hours in day) = 5760 blocks per day
DefaultDowntimeJailDuration = 60 * 10 * time.Second
)

Expand Down
4 changes: 2 additions & 2 deletions x/staking/types/params.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ const (
DefaultHistoricalEntries uint32 = 10000
)

// DefaultMinCommissionRate is set to 0%
var DefaultMinCommissionRate = sdk.ZeroDec()
// DefaultMinCommissionRate is set to .05
var DefaultMinCommissionRate = sdk.NewDecWithPrec(5, 2)

var (
KeyUnbondingTime = []byte("UnbondingTime")
Expand Down