search

celestiaorg / celestia-app

PoS application for the consensus portion of the Celestia network. Built using celestia-core (fork of CometBFT) and the cosmos-sdk
https://celestia.org
Apache License 2.0
330 stars 271 forks source link

Test time-based inflation #1758

Closed rootulp closed 1 year ago

rootulp commented 1 year ago

Context

https://github.com/celestiaorg/celestia-app/pull/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

evan-forbes commented 1 year ago

Additionally time-based inflation isn't battle tested and may introduce new attack vectors.

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?

rootulp commented 1 year ago

Is it correct to say that we are using this mechanism in a similar way to how osmosis is choosing the epoch block?

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.

Like, given a time in the header, we are measuring against another pre-agreed upon time to deterministically trigger some action?

I think it's fair to say yes to this however it isn't exactly the same as how Osmosis chooses the epoch block.

are there any other points that we think are important to note or watch out for?

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).

evan-forbes commented 1 year ago

can we close this?

rootulp commented 1 year ago

Sure, I'll create a separate issue to get x/mint audited by an external team and close this.

Update: created https://github.com/celestiaorg/celestia-app/issues/2085