F/rewards distribution send #96
Conversation
contracts/babylon/src/contract.rs
Outdated
| // Route to babylon over IBC | ||
| let channel = IBC_CHANNEL.load(deps.storage)?; |
There was a problem hiding this comment.
Hmm I guess IBC_CHANNEL is the channel that this contract involves, but not necessarily a channel for IBC transfer. Probably we need to confirm this in some e2e test.
Maybe as a part of the integration, one needs to establish an IBC channel for interchain transfer, and the contract side needs to remember this channel ID in the config, for facilitating the reward mechanism.
There was a problem hiding this comment.
Yes, this was my suspicion as well. Implemented it this way for simplicity. Perhaps we can configure a single channel to do / support both, custom messages and ICS20 transfers?
Will write an e2e test and confirm this in any case.
There was a problem hiding this comment.
Was finally able to e2e test this.
Using a single channel doesn't work, as CosmWasm's IbcMsg::Transfer requires an established channel against the ibctransfer module, not the contract address under wasm.
I think this is a design decision that makes sense, as in general you don't want to establish a channel for every contract that needs / wants to use ICS20 transfers.
Update: Ref.: https://github.com/CosmWasm/cosmwasm/blob/4da538debc73065f9aa8f489895cb54393db0f7c/packages/std/src/ibc.rs#L26-L54
There was a problem hiding this comment.
Using a single channel doesn't work, as CosmWasm's IbcMsg::Transfer requires an established channel against the ibctransfer module, not the contract address under wasm.
Yes that's right, even though we can wire up a IBC transfer packet in the contract side, babylon side's zone concerige won't recognise it.
If we really, really want to let zone concerige handle IBC tansfer, then zone concierge has to invoke IBC transfer module (in particular this code snippet) to handle the IBC transfer packets. But I'm not sure if it's worth it
So I'd recommend we consider consolidating IBC channels as future work, and for now start with having 2 IBC channels. This also creates a clean separation between channels' functionalities following IBC channels' best practices
maurolacy
force-pushed
the
f/rewards-distribution-send
branch
from
961ca90 to
da5dbc2
Compare
maurolacy
changed the base branch from
f/rewards-distribution
to
f/rewards-distribution-2
maurolacy
force-pushed
the
f/rewards-distribution-2
branch
from
934aabc to
5a3698e
Compare
maurolacy
force-pushed
the
f/rewards-distribution-send
branch
from
da5dbc2 to
54e733f
Compare
maurolacy
force-pushed
the
f/rewards-distribution-send
branch
2 times, most recently
from
da78c91 to
e528d44
Compare
maurolacy
force-pushed
the
f/rewards-distribution-send
branch
2 times, most recently
from
9455a91 to
cb8e897
Compare
| @@ -632,14 +632,20 @@ pub fn distribute_rewards(deps: &mut DepsMut, env: &Env) -> Result<(), ContractE | |||
| }; | |||
| // Get the voting power of the active FPS | |||
| let total_voting_power = active_fps.iter().map(|fp| fp.power as u128).sum::<u128>(); | |||
| // Get the rewards to distribute (bank balance of the finality contract) | |||
| // Get the rewards to distribute (bank balance of the finality contract minus already distributed rewards) | |||
| let distributed_rewards = TOTAL_REWARDS.load(deps.storage)?; | |||
There was a problem hiding this comment.
Is it possible to get the total reward by querying the contract's balance, rather than tracking it in a Kv store?
There was a problem hiding this comment.
The problem with this is, that TOTAL_REWARDS are the rounded sum of the distributed rewards over finality providers. They are not necessarily the same. With this impl, we left the remainder in the balance, and it'll be distributed later eventually. Better than send an amount that doesn't match the sum of the distributed rewards, IMO.
…n op slashing msg
This reverts commit 878776c.
maurolacy
force-pushed
the
f/rewards-distribution-send
branch
from
a8e320c to
f42067d
Compare
|
Merging as approved. Please take a look at recent changes (IBC transfer channel info) and comment back in any case. TODOs (custom rewards distribution protocol) will be done in follow-up PRs. |
#97 follow-up. Sends the rewards along with the distribution table to Babylon using ICS-20 with a json payload in the Memo field.