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

fix: deposit channel expiry #3998

Merged
merged 19 commits into from
Sep 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions api/bin/chainflip-broker-api/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use tracing::log;
#[derive(Serialize, Deserialize, Clone)]
pub struct BrokerSwapDepositAddress {
pub address: String,
pub expiry_block: BlockNumber,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change to the api. SDK will probably need to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, linked the companion PR in the description

pub issued_block: BlockNumber,
pub channel_id: ChannelId,
}
Expand All @@ -33,7 +32,6 @@ impl From<chainflip_api::SwapDepositAddress> for BrokerSwapDepositAddress {
fn from(value: chainflip_api::SwapDepositAddress) -> Self {
Self {
address: value.address,
expiry_block: value.expiry_block,
issued_block: value.issued_block,
channel_id: value.channel_id,
}
Expand Down
3 changes: 1 addition & 2 deletions api/bin/chainflip-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ async fn run_cli() -> Result<()> {
let api = StateChainApi::connect(scope, cli_settings.state_chain).await?;
match command_line_opts.cmd {
Broker(BrokerSubcommands::RequestSwapDepositAddress(params)) => {
let SwapDepositAddress { address, expiry_block, .. } = api
let SwapDepositAddress { address, .. } = api
.broker_api()
.request_swap_deposit_address(
params.source_asset,
Expand All @@ -71,7 +71,6 @@ async fn run_cli() -> Result<()> {
)
.await?;
println!("Deposit Address: {address}");
println!("Address expires at block {expiry_block}");
},
LiquidityProvider(
LiquidityProviderSubcommands::RequestLiquidityDepositAddress { asset },
Expand Down
7 changes: 1 addition & 6 deletions api/lib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,6 @@ pub trait GovernanceApi: SignedExtrinsicApi {

pub struct SwapDepositAddress {
pub address: String,
pub expiry_block: state_chain_runtime::BlockNumber,
pub issued_block: state_chain_runtime::BlockNumber,
pub channel_id: ChannelId,
}
Expand Down Expand Up @@ -349,10 +348,7 @@ pub trait BrokerApi: SignedExtrinsicApi {

if let Some(state_chain_runtime::RuntimeEvent::Swapping(
pallet_cf_swapping::Event::SwapDepositAddressReady {
deposit_address,
expiry_block,
channel_id,
..
deposit_address, channel_id, ..
},
)) = events.iter().find(|event| {
matches!(
Expand All @@ -364,7 +360,6 @@ pub trait BrokerApi: SignedExtrinsicApi {
}) {
Ok(SwapDepositAddress {
address: deposit_address.to_string(),
expiry_block: *expiry_block,
issued_block: header.number,
channel_id: *channel_id,
})
Expand Down
58 changes: 22 additions & 36 deletions api/lib/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use cf_primitives::{chains::assets::any, AssetAmount};
use chainflip_engine::state_chain_observer::client::{
chain_api::ChainApi, storage_api::StorageApi,
};
use pallet_cf_ingress_egress::DepositChannelDetails;
use serde::Deserialize;
use state_chain_runtime::PalletInstanceAlias;
use std::{collections::BTreeMap, sync::Arc};
Expand All @@ -15,7 +16,6 @@ pub struct SwapChannelInfo<C: Chain> {
deposit_address: <C::ChainAccount as ToHumanreadableAddress>::Humanreadable,
source_asset: any::Asset,
destination_asset: any::Asset,
expiry_block: state_chain_runtime::BlockNumber,
}

pub struct QueryApi {
Expand Down Expand Up @@ -52,47 +52,33 @@ impl QueryApi {
let block_hash =
block_hash.unwrap_or_else(|| self.state_chain_client.latest_finalized_hash());

let (channel_details, channel_actions, network_environment) = tokio::try_join!(
self.state_chain_client
.storage_map::<pallet_cf_ingress_egress::DepositChannelLookup<
state_chain_runtime::Runtime,
C::Instance,
>, Vec<_>>(block_hash)
.map(|result| {
result.map(|channels| channels.into_iter().collect::<BTreeMap<_, _>>())
}),
self.state_chain_client.storage_map::<pallet_cf_ingress_egress::ChannelActions<
let (channel_details, network_environment) = tokio::try_join!(
self.state_chain_client
.storage_map::<pallet_cf_ingress_egress::DepositChannelLookup<
state_chain_runtime::Runtime,
C::Instance,
>, Vec<_>>(block_hash,),
self.state_chain_client
.storage_value::<pallet_cf_environment::ChainflipNetworkEnvironment<
state_chain_runtime::Runtime,
>>(block_hash),
)?;
>, Vec<_>>(block_hash)
.map(|result| {
result.map(|channels| channels.into_iter().collect::<BTreeMap<_, _>>())
}),
self.state_chain_client
.storage_value::<pallet_cf_environment::ChainflipNetworkEnvironment<state_chain_runtime::Runtime>>(
block_hash
),
)?;

Ok(channel_actions
Ok(channel_details
.iter()
.filter_map(|(address, action)| {
match action {
pallet_cf_ingress_egress::ChannelAction::Swap { destination_asset, .. } |
pallet_cf_ingress_egress::ChannelAction::CcmTransfer {
destination_asset,
..
} => Some(destination_asset),
_ => None,
}
.and_then(|destination_asset| {
channel_details.get(address).map(|details| {
(destination_asset, details.deposit_channel.clone(), details.expires_at)
})
})
.map(|(&destination_asset, deposit_channel, expiry)| SwapChannelInfo {
.filter_map(|(_, DepositChannelDetails { action, deposit_channel, .. })| match action {
pallet_cf_ingress_egress::ChannelAction::Swap { destination_asset, .. } |
pallet_cf_ingress_egress::ChannelAction::CcmTransfer {
destination_asset, ..
} => Some(SwapChannelInfo {
deposit_address: deposit_channel.address.to_humanreadable(network_environment),
source_asset: deposit_channel.asset.into(),
destination_asset,
expiry_block: expiry,
})
destination_asset: *destination_asset,
}),
_ => None,
})
.collect::<Vec<_>>())
}
Expand Down
2 changes: 1 addition & 1 deletion bouncer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"prettier:write": "prettier --write ."
},
"dependencies": {
"@chainflip-io/cli": "^0.1.3",
"@chainflip-io/cli": "^0.1.4",
"@polkadot/api": "10.7.2",
"@polkadot/keyring": "12.2.1",
"@polkadot/util": "12.2.1",
Expand Down
8 changes: 4 additions & 4 deletions bouncer/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

47 changes: 0 additions & 47 deletions bouncer/shared/lp_deposit_expiry.ts

This file was deleted.

2 changes: 0 additions & 2 deletions bouncer/tests/all_concurrent_tests.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/usr/bin/env -S pnpm tsx
import { testLpDepositExpiry } from '../shared/lp_deposit_expiry';
import { testAllSwaps } from '../shared/swapping';
import { testEthereumDeposits } from '../shared/ethereum_deposits';
import { runWithTimeout, observeBadEvents } from '../shared/utils';
Expand All @@ -13,7 +12,6 @@ async function runAllConcurrentTests() {

await Promise.all([
testAllSwaps(),
testLpDepositExpiry(),
testEthereumDeposits(),
testFundRedeem('redeem'),
testMultipleMembersGovernance(),
Expand Down
13 changes: 0 additions & 13 deletions bouncer/tests/lp_deposit_expiry.ts

This file was deleted.

5 changes: 5 additions & 0 deletions engine/src/witness/btc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@ mod tests {
btc::{deposit_address::DepositAddress, ScriptPubkey},
DepositChannel,
};
use pallet_cf_ingress_egress::ChannelAction;
use sp_runtime::AccountId32;

fn fake_transaction(tx_outs: Vec<TxOut>) -> Transaction {
Transaction {
Expand All @@ -218,6 +220,9 @@ mod tests {
asset: btc::Asset::Btc,
state: DepositAddress::new([0; 32], 1),
},
action: ChannelAction::<AccountId32>::LiquidityProvision {
lp_account: AccountId32::new([0xab; 32]),
},
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ where
// FOr a given header we only witness addresses opened at or before the header, the set of
// addresses each engine attempts to witness at a given block is consistent
fn addresses_for_header(index: Inner::Index, addresses: &Addresses<Inner>) -> Addresses<Inner> {
addresses.iter().filter(|details| details.opened_at <= index).cloned().collect()
addresses
.iter()
.filter(|details| details.opened_at <= index && index <= details.expires_at)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer if we use an exclusive upper bound here.

This is more consistent with ranges in general (opened_at..expires_at).

Also it seems odd that we can submit something at the block in which it expires.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inclusive end is consistent with how we witness everything else, the external chain block number for the vault rotation, is witnessed by the previous authority set, i.e. to the previous set, it's inclusive of the range they witness, which is why I made it inclusive

.cloned()
.collect()
}

async fn get_chain_state_and_addresses<StateChainClient: StorageApi + Send + Sync + 'static>(
Expand Down
4 changes: 3 additions & 1 deletion state-chain/cf-integration-tests/src/mock_runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,11 @@ impl ExtBuilder {
environment: Default::default(),
liquidity_pools: Default::default(),
swapping: Default::default(),
liquidity_provider: Default::default(),
system: Default::default(),
transaction_payment: Default::default(),
bitcoin_ingress_egress: Default::default(),
polkadot_ingress_egress: Default::default(),
ethereum_ingress_egress: Default::default(),
})
}
}
Loading