-
Notifications
You must be signed in to change notification settings - Fork 28
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 #303
Open
github-actions
wants to merge
69
commits into
base/consumer-chain-support
Choose a base branch
from
main
base: base/consumer-chain-support
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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
Updates cosmos math. [Issue](GHSA-7225-m954-23v7)
(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`
Closes #259. Using gauge is not easy to fix the issue as we don't know the number of fps being jailed. Therefore, this PR fixed the issue by adding a separate counter for unjailed fps.
- fixes genesis validation
- Fixes config linter config
- enables more linters from golang ci lint - fixes some low hanging fruits
Adds initial scaffold for tests which enable full execution of the block by comet-bft utilities: - `BlockExecutor` Those are test can be used in various scenarios: - e2e application test asserting that after certain ordered operations, we reached expected state - deterministic execution tests i.e execute some random operations and replay them and check that app hash matches - early block execution tests (if we introduce other backends than memdb)
- Helps to convert keys from pub key ```shell babylond debug pubkey-raw "A2h06lj/GAPW1RvJE3HU9yetMGCR9K7a+tcwwTYT8t7z" Parsed key as secp256k1 Address: 3565AEBC1D0CF089A4C2B04500EFE5007701604E JSON (base64): {"type":"tendermint/PubKeySecp256k1","value":"A2h06lj/GAPW1RvJE3HU9yetMGCR9K7a+tcwwTYT8t7z"} Bech32 Acc: bbnpub1addwnpepqd58f6jcluvq84k4r0y3xuw57un66vrqj862akh66ucvzdsn7t00x5n57a8 Bech32 Validator Operator: bbnvaloperpub1addwnpepqd58f6jcluvq84k4r0y3xuw57un66vrqj862akh66ucvzdsn7t00x4p4spq Bech32 Validator Consensus: BIP340 Hex: 036874ea58ff1803d6d51bc91371d4f727ad306091f4aedafad730c13613f2def3 ```
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
- enable more lints
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 <[email protected]>
`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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR synchronizes changes from main to base/consumer-chain-support.