Skip to content

Commit

Permalink
fix(market): batch published deals event (#579)
Browse files Browse the repository at this point in the history
  • Loading branch information
jmg-duarte authored Nov 22, 2024
1 parent 0ce3b67 commit 0865026
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 52 deletions.
Binary file modified cli/artifacts/metadata.scale
Binary file not shown.
18 changes: 14 additions & 4 deletions cli/polka-storage-provider/server/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,26 @@ impl StorageProviderRpcServer for RpcServerState {

let published_deals = result
.events
.find::<storagext::runtime::market::events::DealPublished>()
.collect::<Result<Vec<_>, _>>()
.find_first::<storagext::runtime::market::events::DealsPublished>()
.map_err(|err| RpcError::internal_error(err, None))?;
let Some(published_deals) = published_deals else {
return Err(RpcError::internal_error(
"failed to find any published deals",
None,
));
};

// We currently just support a single deal and if there's no published deals,
// an error MUST've happened
debug_assert_eq!(published_deals.len(), 1);
debug_assert_eq!(published_deals.deals.0.len(), 1);

// We always publish only 1 deal
let deal_id = published_deals[0].deal_id;
let deal_id = published_deals
.deals
.0
.first()
.expect("we only support a single deal")
.deal_id;

let commitment = Commitment::from_cid(&piece_cid).map_err(|e| {
RpcError::invalid_params(
Expand Down
27 changes: 20 additions & 7 deletions cli/polka-storage/storagext/src/runtime/display/market.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,27 @@ impl std::fmt::Display for Event {
who, amount
))
}
Event::DealPublished {
deal_id,
client,
Event::DealsPublished {
provider,
} => f.write_fmt(format_args!(
"Deal Published: {{ deal_id: {}, provider_account: {}, client_account: {} }}",
deal_id, provider, client,
)),
deals
} => {
// This should show something like
// Deals Published: {
// provider_account: ...,
// deals: [
// { client_account: ..., deal_id: ... },
// { client_account: ..., deal_id: ... },
// ]
// }
f.write_fmt(format_args!(
"Deal Published: {{\n provider_account: {},\n deals: [\n",
provider
))?;
for deal in deals.0.iter() {
f.write_fmt(format_args!(" {{ client_account: {}, deal_id: {} }},\n", deal.client, deal.deal_id))?;
}
f.write_str(" ]\n}")
}
Event::DealActivated {
deal_id,
client,
Expand Down
8 changes: 8 additions & 0 deletions cli/polka-storage/storagext/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ pub mod display;
path = "pallet_market::pallet::DealState",
derive = "::serde::Deserialize"
),
derive_for_type(
path = "pallet_market::pallet::PublishedDeal",
derive = "::serde::Deserialize"
),
// impl Serialize
derive_for_type(
path = "pallet_market::pallet::BalanceEntry",
Expand Down Expand Up @@ -112,6 +116,10 @@ pub mod display;
path = "pallet_market::pallet::DealState",
derive = "::serde::Serialize"
),
derive_for_type(
path = "pallet_market::pallet::PublishedDeal",
derive = "::serde::Serialize"
),
)]
mod polka_storage_runtime {}

Expand Down
7 changes: 4 additions & 3 deletions maat/tests/real_world.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,15 @@ async fn publish_storage_deals<Keypair>(

for event in deal_result
.events
.find::<storagext::runtime::market::events::DealPublished>()
.find::<storagext::runtime::market::events::DealsPublished>()
{
let event = event.unwrap();
tracing::debug!(?event);

assert_eq!(event.client, alice.account_id().clone().into());
assert_eq!(event.provider, charlie.account_id().clone().into());
assert_eq!(event.deal_id, 0); // first deal ever
assert_eq!(event.deals.0.len(), 1);
assert_eq!(event.deals.0[0].client, alice.account_id().clone().into());
assert_eq!(event.deals.0[0].deal_id, 0); // first deal ever
}
}

Expand Down
43 changes: 34 additions & 9 deletions pallets/market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,13 +362,7 @@ pub mod pallet {
who: T::AccountId,
amount: BalanceOf<T>,
},
/// Deal has been successfully published between a client and a provider.
DealPublished {
deal_id: DealId,
client: T::AccountId,
provider: T::AccountId,
},
// Deal has been successfully activated.
/// Deal has been successfully activated.
DealActivated {
deal_id: DealId,
client: T::AccountId,
Expand Down Expand Up @@ -404,11 +398,35 @@ pub mod pallet {
client: T::AccountId,
provider: T::AccountId,
},

/// Batch of published deals.
DealsPublished {
provider: T::AccountId,
deals: BoundedVec<PublishedDeal<T>, T::MaxDeals>,
},
}

/// Utility type to ensure that the bound for deal settlement is in sync.
pub type MaxSettleDeals<T> = <T as Config>::MaxDeals;

#[derive(TypeInfo, Encode, Decode, Clone, PartialEq)]
pub struct PublishedDeal<T: Config> {
pub client: T::AccountId,
pub deal_id: DealId,
}

impl<T> core::fmt::Debug for PublishedDeal<T>
where
T: Config,
{
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("PublishedDeal")
.field("deal_id", &self.deal_id)
.field("client", &self.client)
.finish()
}
}

/// The data part of the event pushed when the deal is successfully settled.
#[derive(TypeInfo, Encode, Decode, Clone, PartialEq)]
pub struct SettledDealData<T: Config> {
Expand Down Expand Up @@ -890,6 +908,8 @@ pub mod pallet {
let (valid_deals, total_provider_lockup) =
Self::validate_deals(provider.clone(), deals, current_block)?;

let mut published_deals = BoundedVec::new();

// Lock up funds for the clients and emit events
for deal in valid_deals.into_iter() {
// PRE-COND: always succeeds, validated by `validate_deals`
Expand All @@ -913,9 +933,9 @@ pub mod pallet {
Proposals::<T>::insert(deal_id, deal.clone());

// Only deposit the event after storing everything
Self::deposit_event(Event::<T>::DealPublished {
// force_push is ok since the bound is the same as the input one
published_deals.force_push(PublishedDeal {
client: deal.client,
provider: provider.clone(),
deal_id,
});
}
Expand All @@ -924,6 +944,11 @@ pub mod pallet {
// PRE-COND: always succeeds, validated by `validate_deals`
lock_funds::<T>(&provider, total_provider_lockup)?;

Self::deposit_event(Event::<T>::DealsPublished {
deals: published_deals,
provider,
});

Ok(())
}
}
Expand Down
69 changes: 40 additions & 29 deletions pallets/market/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ use crate::{
mock::*,
pallet::{lock_funds, slash_and_burn, unlock_funds},
ActiveDealState, BalanceEntry, BalanceTable, Config, DealSettlementError, DealState,
DealsForBlock, Error, Event, PendingProposals, Proposals, SectorDeals, SectorTerminateError,
SettledDealData,
DealsForBlock, Error, Event, PendingProposals, Proposals, PublishedDeal, SectorDeals,
SectorTerminateError, SettledDealData,
};
#[test]
fn initial_state() {
Expand Down Expand Up @@ -391,10 +391,12 @@ fn publish_storage_deals_fails_different_providers() {
));
assert_eq!(
events(),
[RuntimeEvent::Market(Event::<Test>::DealPublished {
deal_id: 0,
client: account::<Test>(ALICE),
[RuntimeEvent::Market(Event::<Test>::DealsPublished {
provider: account::<Test>(PROVIDER),
deals: bounded_vec!(PublishedDeal {
deal_id: 0,
client: account::<Test>(ALICE),
})
})]
);
});
Expand All @@ -421,10 +423,12 @@ fn publish_storage_deals_fails_client_not_enough_funds_for_second_deal() {
));
assert_eq!(
events(),
[RuntimeEvent::Market(Event::<Test>::DealPublished {
deal_id: 0,
client: account::<Test>(ALICE),
[RuntimeEvent::Market(Event::<Test>::DealsPublished {
provider: account::<Test>(PROVIDER),
deals: bounded_vec!(PublishedDeal {
deal_id: 0,
client: account::<Test>(ALICE),
})
})]
);
});
Expand Down Expand Up @@ -453,10 +457,12 @@ fn publish_storage_deals_fails_provider_not_enough_funds_for_second_deal() {
));
assert_eq!(
events(),
[RuntimeEvent::Market(Event::<Test>::DealPublished {
deal_id: 0,
client: account::<Test>(ALICE),
[RuntimeEvent::Market(Event::<Test>::DealsPublished {
provider: account::<Test>(PROVIDER),
deals: bounded_vec!(PublishedDeal {
deal_id: 0,
client: account::<Test>(ALICE),
})
})]
);
});
Expand All @@ -483,10 +489,12 @@ fn publish_storage_deals_fails_duplicate_deal_in_message() {
));
assert_eq!(
events(),
[RuntimeEvent::Market(Event::<Test>::DealPublished {
deal_id: 0,
client: account::<Test>(ALICE),
[RuntimeEvent::Market(Event::<Test>::DealsPublished {
provider: account::<Test>(PROVIDER),
deals: bounded_vec!(PublishedDeal {
deal_id: 0,
client: account::<Test>(ALICE),
})
})]
);
});
Expand All @@ -508,10 +516,12 @@ fn publish_storage_deals_fails_duplicate_deal_in_state() {
));
assert_eq!(
events(),
[RuntimeEvent::Market(Event::<Test>::DealPublished {
deal_id: 0,
client: account::<Test>(ALICE),
[RuntimeEvent::Market(Event::<Test>::DealsPublished {
provider: account::<Test>(PROVIDER),
deals: bounded_vec!(PublishedDeal {
deal_id: 0,
client: account::<Test>(ALICE),
})
})]
);
assert_noop!(
Expand Down Expand Up @@ -583,18 +593,19 @@ fn publish_storage_deals() {

assert_eq!(
events(),
[
RuntimeEvent::Market(Event::<Test>::DealPublished {
deal_id: alice_deal_id,
client: account::<Test>(ALICE),
provider: account::<Test>(PROVIDER),
}),
RuntimeEvent::Market(Event::<Test>::DealPublished {
deal_id: bob_deal_id,
client: account::<Test>(BOB),
provider: account::<Test>(PROVIDER),
}),
]
[RuntimeEvent::Market(Event::<Test>::DealsPublished {
provider: account::<Test>(PROVIDER),
deals: bounded_vec!(
PublishedDeal {
deal_id: alice_deal_id,
client: account::<Test>(ALICE),
},
PublishedDeal {
deal_id: bob_deal_id,
client: account::<Test>(BOB),
}
)
}),]
);
assert!(PendingProposals::<Test>::get().contains(&alice_hash));
assert!(PendingProposals::<Test>::get().contains(&bob_hash));
Expand Down

0 comments on commit 0865026

Please sign in to comment.