[PROTO-2787] Make investor token unlock amounts strictly monotonically increasing

[relyt29]

Purpose of Changes and their Description

A bug was discovered during conversation with @jmdkastro around tokenomics. We take a module parameter called blocksPerMonth to make the chain aware of the expected number of blocks per month. The actual blocks for every month varies from month to month and also due to network conditions. On testnet it has been not very stable. The expectation is that maintainers of the network would periodically update the blocksPerMonth parameter to keep it up to date, however, depending on the change of blocksPerMonth, that could actually compute different values for the tokenomics, potentially giving away too many tokens, leading to a higher than expected inflation rate.

The inflation rate is controlled by the target emission rate (e_i in the whitepaper). The target emission rate is inversely dependent upon the circulating supply of the token. The circulating supply of the token is dependent upon the investor token unlock schedule, which is time dependent - we expect a 3 year vesting with 1 year cliff unlock cycle (no tokens unlock until the first year). This unlock logic is dependent on the blocksPerMonth and if the blocksPerMonth changes in strange ways, it could lead to a too-high inflation rate.

For example, lets say on month 1, we set the blocksPerMonth to 5 blocks per month, but in real life, the blocks are being produced 1 block per second. That means that every second, the chain thinks that 1/5 of a month has occurred. So after 60 seconds (5 seconds for 5 blocks per month * 12 months), the token unlock schedule will start and the circulating supply will start to think that the circulating supply has gone up by the token investor unlock amount.

Later, someone comes along and sets the blocksPerMonth to a huge value say 600000000. Because the logic for the vesting supply has no memory of previous token unlock events, the chain now thinks that the circulating supply is lower than it thought before. Accordingly, it adjusts the inflation rate to be higher, resulting in a too-high inflation rate. The correct thing to do instead is keep some memory of previous circulating supply changes so that you always err on the side of a too-low inflation rate, rather than too high.

@jmdkastro came up with an elegant "integral math" solution to this where you basically, every block, take some delta change in the vested token amount and accumulate it over time until the full unlock has been unlocked.

That is totally functional but when I went to go implement it I observed that the actual on-chain tokenomics as currently implemented are stepwise anyways, since the token unlocks are written to happen on the first of a month when the unlock is supposed to happen, AND we only recalculate the target emission rate on a monthly basis anyways (for performance reasons). So no matter if the "integral math" was implemented it would still be a stepwise function ANYWAYS.

So given that, I implemented a new keeper variable called "monthsUnlocked" which is meant to track the number of months that have passed since genesis. This is a value that is coerced to be between 0 and 36, and is stored in a strictly ever increasing value. This way, if an unlock of tokens ever happens, that unlock is not revertable in the logic of the chain. If the chain thinks that 200 blocks per second are being produced and that its time to unlock a bunch of tokens, and then later the chain thinks that only 1 block is being produced per second, when the chain goes to do the logic for token unlocks, it takes the stored value from the past, rather than the calculated value at the moment of what it thinks the token unlock should be. This is basically the same thing as the integral math solution but stepwise on a months cadence to match our existing on-chain logic, which makes for a smaller diff and faster integration on chain.

Also while doing this I had to update the query service to return this additional new piece of data, and while I was doing that, the protobuf linter complained about the query service being called "QueryServer" instead of "QueryServiceServer" so theres a bit of boilerplate around that as well in this PR

Link(s) to Ticket(s) or Issue(s) resolved by this PR

PROTO-2787

Are these changes tested and documented?

We have an existing test suite for this in emissions_test.go, I have tweaked it to make it appropriately test the new code. I have not made any changes to the documentation as this is kind of a minor change.

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf Linter / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	⏩ skipped	Dec 4, 2024, 7:00 PM

[xmariachi]

A comment on the solution ab "estimating the past"

[xmariachi]

lgtm

[guilherme-brandao]

lgtm! just a minor (optional) suggestion

[kpeluso]

the concepts of monthsAlreadyUnlocked and updatedMonthsUnlocked seems so similar, yet are manipulated in 2 different scopes. for this, I'm not going to block, as there is some overlapping control flow (to calc updatedMonthsUnlocked and the other values, obvi) but I think it'd be cleaner to separate this logic from the compute of vested tokens