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.

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

Sync from main to base/consumer-chain-support #400

Merged
merged 76 commits into from
Jan 14, 2025

Conversation

github-actions[bot]
Copy link

This PR synchronizes changes from main to base/consumer-chain-support.

huynaism and others added 30 commits November 12, 2024 17:46
Root cause of
babylonlabs-io/finality-provider#121. If
response is nil due to error, will cause panic
Closes #263. In particular,
- ensure `checkpointFinalizationTimeout` can never be changed in the
update params handler of the btccheckpoint module
- ensure `minUnbondingTime` can not be set to a value less than or equal
to `checkpointFinalizationTimeout` in the update params handler of the
btcstaking module
- allows `unbondingTime` of a delegation to be set to a value equals to
`minUnbondingTime`
This PR introduces the `ResumeFinalityProposal` and implements the
handler, which is part of
[ADR-32](babylonlabs-io/pm#95). This part is
quite independent and the algorithm of choosing finality providers to
jail can be implemented in the future
(docker)resolve Dockerfile issue & fix CVEs
The bug is caused due to setting `params.MinStakingTime` to the
btcstaking module's keeper method `VerifyInclusionProofAndGetHeight`
other than `params.MinUnbondingTime`. In the fix, we remove
`minUnbondingTime` from the parameter list of
`VerifyInclusionProofAndGetHeight` as it should retrieve the parameter
within the method.

This PR also added comprehensive tests for
`VerifyInclusionProofAndGetHeight`
KonradStaniec and others added 26 commits December 12, 2024 04:39
Pr changes way how babylon choses parameters for pre-approval flow

- parameters will be chosen by BTC LC tip
- to avoid attacks in which somebody will first send tx to BTC and then
report it as EOI, additional validation during activation is added to
check that staking transaction is included after the BTC LC tip during
delegation creation.
Add determinism test for whole staking activation flow
pending/verified/active

Next steps:
- finalizing epochs
- finality provider voting and randomness registration
The voting power table is a map which causes random order of processing
jailing events. If more than one fps jailed at one block,
non-determinism could happen. The PR fixed this by introducing ordered
voting power table which retains the order from interating the
`votingPowerBbnBlockHeightStore`
- Add fuzzing test case for jailing being deterministc
- Move `PrivateSigner` to specific package
- Created vars for constant used Mod accounts in appparams
`EndBlocker` -> `BeginBlocker` in BeginBlocker section
There has been an issue relative to
babylonlabs-io/finality-provider#231 and this pr
adds `MsgSetWithdrawAddress` to the necessary codec file plus adds it to
`types/msg.go`
- Add new structures to track BTC delegation rewards and lazily
distribute
- Add new event `BTCDelegationStatus_EXPIRED` for BTC delegations that
indicates a BTC delegation is no longer contributing to security
- Important files diff for review
  -  `proto/babylon/incentive/rewards.proto`
  -  `x/finality/keeper/power_dist_change.go`
  - `x/incentive/keeper/btc_staking_gauge.go`
  - `x/incentive/keeper/grpc_query.go`
  - `x/incentive/keeper/msg_server.go`
  - `x/incentive/keeper/reward_tracker.go`
  - `x/incentive/keeper/reward_tracker_store.go`
  - `x/btcstaking/keeper/btc_delegations.go`
  - `x/btcstaking/keeper/msg_server.go`
  - `x/btcstaking/types/btc_delegation.go`


## Benchmarks

Command to generate pprof `go tool pprof -http=:8080
http://localhost:6060/debug/pprof/profile\?seconds\=300`

- Main branch that iterates over delegations to distribute rewards with
163k BTC delegations, took 15.52s (13.1%) of the time during the
300seconds interval used to generate the pprof


![image](https://github.com/user-attachments/assets/83746927-87ec-421b-87ce-bae16c7c3000)

- Current branch `rafilx/improve-performance-btc-dist` with 100k BTC
delegations, took 6.73s (1.8%) of the time during the 300seconds
interval


![image](https://github.com/user-attachments/assets/ddf694a9-ed26-4a21-8fed-03a91d1e8450)


Note that the percentage of the time used that is ~7x lower should be
considered instead of the time, the benchmark from `main` was running in
the benchmark server 48 processors and 256GB of RAM while the one in the
current branch I run locally with 24 processors and 96GB of RAM.... So
the percentage of the time usage instead of seconds should be the one in
comparison

> Thanks @Lazar955  for help with benchmark
> The function used to show the difference is the `TallyBlocks` because
the `RewardBTCStaking` does not appear in the pprof of the current
branch demonstrating that the time spent there is negligible to the
overall period
Implements babylonlabs-io/pm#145 including:
- Rewards are calculated with a timeout `finality_sig_timeout`
- Rewards should not be assigned to finality providers that have not
voted for the block when calculating rewards.
- Upgrades parameters in upgrade for testnet launch

---------

Co-authored-by: Filippos Malandrakis <philmln@protonmail.com>
`EventPowerDistUpdate_SlashedFp` and
[`RewardBTCStaking`](https://github.com/babylonlabs-io/babylon/blob/b132d16483d2e12ac68be74a524a4b7121edac65/x/incentive/keeper/btc_staking_gauge.go#L19)
are called at different blocks.
The rewards are being called a few blocks behind and if the slashed fp
is pruned in BTC reward tracker data, it fails to find and panics

On babylond-deployments an fp is being slashed at block height 46, the
call of `RewardBTCStaking` happens at Babylon block 48 but is called to
distribute rewards of voting power table from block 44 (which didn't had
the FP slashed yet) which then checks for fp rewards for the FP that was
pruned at the slashing processed event.

```
6:04PM INF finalizing commit of block hash=153E869F7B0056A9CD8208301F2019FFACA2BEFBBB35AEADF70DCA14C15F3972 height=47 module=consensus num_txs=2 root=62B5F025459DEEBE2EE6F5D869C5BCBE903299FF80825FB9DE62551E6B725D52
 ERR CONSENSUS FAILURE!!! err="failed to add fp rewards for btc delegation bbn1wwkaq6z3kdkekm7ltwxng4ftvzux8gzp2xjx8d at height 44: finality provider current rewards not found
``` 


```shell
$~ babylond q btcstaking finality-providers -o json | jq
...
    {
      "description": {
        "moniker": "Finality Provider 0",
        "identity": "",
        "website": "",
        "security_contact": "",
        "details": ""
      },
      "commission": "0.050000000000000000",
      "addr": "bbn1wwkaq6z3kdkekm7ltwxng4ftvzux8gzp2xjx8d",
      "btc_pk": "6ba387c315dc6105ec02b50684593505103a9f13452d3a597c16ac2f22b924dc",
      "pop": {
        "btc_sig_type": "BIP340",
        "btc_sig": "Q3PcfiKT0fwWOVx95rjV+mQvI/juTbYIV84Hfz2NAmQByiHRXz8OWnFbyugJgOrd/TrqR+6BOgtr10bZUFDFbA=="
      },
      "slashed_babylon_height": "46",
      "slashed_btc_height": 131,
      "height": "46",
      "jailed": false,
      "highest_voted_height": 45
    },
```


Basically, this means we are processing the rewards a few blocks down
the road after the slash and then it is not possible to prune the data
for reward tracker until that fp slashed height is processed.

The fix for the time being: do nothing at the btc reward tracker
structures if some fp gets slashed just continue to store that data. In
the future during the issue
#362 we can properly
handle the pruning of slashed finality provider data
#350 Introduced regression
due to change from if/else to switch. `break` which earlier was jumping
out of the loop, after change would only exit the `switch` statement.

Fix:
- add `finalizationLoop` label and use it during `break`
- add proper test to check that finalization in consecutive.
Prior to this fix, if we had an unfinalized block it would stuck to it
as a next block to be rewarded


By using `continue` instead of `break`, if there is any finalized block
in the interval (`nextBlockToBeRewarded` and `targetHeight`) it will
give out rewards and update the next block to be rewarded
- fixes epoching ante handler that returned early from ante handler even
in case of success validation of staking message
-backport of changelog changes for v1.0.0-rc.3
@filippos47 filippos47 merged commit cbfb705 into base/consumer-chain-support Jan 14, 2025
56 of 57 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.