F/rewards generation #61
Conversation
maurolacy
force-pushed
the
f/rewards-generation
branch
from
d259610
to
a65782b
Compare
| @@ -106,6 +107,8 @@ func (s *BabylonSDKTestSuite) Test1ContractDeployment() { | |||
| msgUpdateParams := &bbntypes.MsgUpdateParams{ | |||
| Authority: s.ConsumerApp.BabylonKeeper.GetAuthority(), | |||
| Params: bbntypes.Params{ | |||
| FinalityInflationRate: sdkmath.LegacyNewDecWithPrec(7, 2), | |||
| BlocksPerYear: 60 * 60 * 25 * 365 / 5, // 5 seconds per block | |||
There was a problem hiding this comment.
This can be obtained from x/mint's MintKeeper so the Babylon SDK module does not need to have it
There was a problem hiding this comment.
OK, will refactor.
| type ( | ||
| CustomMsg struct { | ||
| MintRewards *MintRewardsMsg `json:"mint_rewards,omitempty"` | ||
| } | ||
| // MintRewardsMsg mints block rewards to the staking contract | ||
| MintRewardsMsg struct{} | ||
| ) |
There was a problem hiding this comment.
Since these will be triggered upon BeginBlock anyway, wdyt about doing the inflation entirely in BeginBlock rather than going through the "module -> contract -> module" workflow?
There was a problem hiding this comment.
Did it this way because I want rewards to be minted only on block finalisation. See babylonlabs-io/babylon-contract#80.
Probably there are simpler ways to detect we have an active FP set that is signing blocks, but this is the strongest one I can see. And yes, it depends on the finality contract.
There was a problem hiding this comment.
Did it this way because I want rewards to be minted only on block finalisation. See babylonlabs-io/babylon-contract#80.
This is a product design that I'm not entirely sure. IMO, since every block is expected to be finalised anyway, it's okay to mint tokens upon every block. If we don't want to distribute tokens when no block is finalised, we could instead distribute tokens upon finalising a block. Wdyt?
| } | ||
|
|
||
| func (h CustomMsgHandler) handleTestMsg(ctx sdk.Context, actor sdk.AccAddress, testMsg *contract.TestMsg) ([]sdk.Event, [][]byte, [][]*codectypes.Any, error) { | ||
| return []sdk.Event{}, nil, nil, nil | ||
| func (h CustomMsgHandler) handleMintRewardsMsg(ctx sdk.Context, actor sdk.AccAddress, _ *contract.MintRewardsMsg) ([]sdk.Event, [][]byte, [][]*codectypes.Any, error) { |
There was a problem hiding this comment.
Yeah basically this can be invoked inside BeginBlock
There was a problem hiding this comment.
I'm not so sure. What if the FP set is inactive? What if they are not submitting signatures at all, or not enough signatures for finalisation? Etc.
There was a problem hiding this comment.
What if the FP set is inactive? What if they are not submitting signatures at all, or not enough signatures for finalisation? Etc.
We could distribute tokens upon finalising a block. Basically this can be approached in the reward distribution but not reward generation.
There was a problem hiding this comment.
What is the problem with this design? Do you think it's too complicated?
In my opinion, it's better to mint rewards related to block finalisation only when such blocks are being effectively finalised. But I can refactor, if we agree it's worth it.
There was a problem hiding this comment.
Those contract <-> blockchain messages are very efficient by the way.
There was a problem hiding this comment.
This is in fact being executed at the end blocker. The current sequence is blockchain -> end_block -> contract -> blockchain.
There was a problem hiding this comment.
As per offline discussion, it is necessary to mint tokens only upon finalising a block. If the protocol mints tokens only upon BeginBlock, then it's possible that the finality contract admin moves all the inflation at wish, e.g., rugpull, bridging it to other contracts, etc.. This is a security critical issue. Babylon does not have this problem because it 1) the inflation is in the fee collector account controlled by gov prop, rather than sth like a contract admin, and 2) people only recognise BTC stake finalised prefix as finalised, but this might not be the case in consumer chains.
There was a problem hiding this comment.
After discussion, closed in favour of #62.
There was a problem hiding this comment.
Could we document this in a ADR?
There was a problem hiding this comment.
Yes. ADR is to come.
Thin layer impl of the finalisation block rewards generation. Supersedes / replaces #61
Implements finality block reward minting based on inflation rate target, expected number of blocks, and total supply.