-
Notifications
You must be signed in to change notification settings - Fork 278
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
Test time-based inflation #1758
Comments
this is a good point and testing this logic is definitely critical. Is it correct to say that we are using this mechanism in a similar way to how osmosis is choosing the epoch block? Like, given a time in the header, we are measuring against another pre-agreed upon time to deterministically trigger some action? If not, can we document how it differs here for reviewers/auditors? are there any other points that we think are important to note or watch out for? precision or format perhaps? |
### Description Part of #1570 Implement the strict inflation schedule specified in #1584 ### Remaining tasks - [x] implement inflation rate change based on block timestamp rather than block height - [x] set genesis time in x/mint state - [x] move `MintDenom` from param to `x/mint` state - [x] remove `TestRandomizedGenState` - [x] investigate why `TestQueryGRPC` / other integration tests are never run - [x] write a test that 1. starts a new chain, 2. verifies state of x/mint module for first few blocks, 3. skips ahead to a block one year later, 4. verifies inflation rate went down a bit - [x] remove `Params` entirely - [x] annual provisions for a year should be based on total supply at the beginning of that year - [x] for time-based inflation, calculate block provision as current block timestamp - previous block timestamp = seconds elapsed / seconds in a year * annual provisions for a year - [x] remove `BlocksPerYear` - [x] elaborate in README.md ### FLUPs - #1755 - #1757 - #1758 - #1756 --------- Co-authored-by: Rootul P <rootulp@gmail.com> Co-authored-by: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Co-authored-by: Callum Waters <cmwaters19@gmail.com>
As far as I can tell, Osmosis mints new tokens due to inflation per epoch block that occurs once per day. The block is selected based on the x/epochs module. The x/epochs signals that an epoch has "ticked" when the block timestamp exceeds an individual time counter's start time + duration. It is generalized to handle chain downtime such that when the chain resumes, a ticker will catch-up by repeatedly ticking for successive blocks, even if a day hasn't elapsed per block. Our x/mint module performs time based calculation strictly on a yearly basis here. It isn't generalized to support arbitrary time durations so the implementation is quite smaller (see Osmosis x/epoch BeginBlocker). Since each block's timestamp is compared against the genesis timestamp, I don't expect there to be issues with the calculation if there is chain downtime.
I think it's fair to say yes to this however it isn't exactly the same as how Osmosis chooses the epoch block.
Yes auditors should pay attention to time precision. They should also explore time boundaries (i.e. the block timestamps near year boundaries). They can also explore edge cases like timestamps before genesis time or extremely far in the future or timestamps that aren't chronologically increasing (although CometBFT may already enforce this isn't possible). |
can we close this? |
Sure, I'll create a separate issue to get Update: created #2085 |
### Description Part of celestiaorg/celestia-app#1570 Implement the strict inflation schedule specified in celestiaorg/celestia-app#1584 ### Remaining tasks - [x] implement inflation rate change based on block timestamp rather than block height - [x] set genesis time in x/mint state - [x] move `MintDenom` from param to `x/mint` state - [x] remove `TestRandomizedGenState` - [x] investigate why `TestQueryGRPC` / other integration tests are never run - [x] write a test that 1. starts a new chain, 2. verifies state of x/mint module for first few blocks, 3. skips ahead to a block one year later, 4. verifies inflation rate went down a bit - [x] remove `Params` entirely - [x] annual provisions for a year should be based on total supply at the beginning of that year - [x] for time-based inflation, calculate block provision as current block timestamp - previous block timestamp = seconds elapsed / seconds in a year * annual provisions for a year - [x] remove `BlocksPerYear` - [x] elaborate in README.md ### FLUPs - celestiaorg/celestia-app#1755 - celestiaorg/celestia-app#1757 - celestiaorg/celestia-app#1758 - celestiaorg/celestia-app#1756 --------- Co-authored-by: Rootul P <rootulp@gmail.com> Co-authored-by: Evan Forbes <42654277+evan-forbes@users.noreply.github.com> Co-authored-by: Callum Waters <cmwaters19@gmail.com>
Context
#1578
Problem
The new
x/mint
module hasn't been used in production and may contain code errors. Additionally time-based inflation isn't battle tested and may introduce new attack vectors.Proposal
x/mint
module #1790x/mint
module #2085The text was updated successfully, but these errors were encountered: