Skip to content

Commit

Permalink
Fix UX Issues (#880)
Browse files Browse the repository at this point in the history 
## Describe your changes
1. Solves Polkadex-Substrate/orderbook#872
2. Swap deposit tokens for ED in PDEX automatically
3. Ability to pay withdrawal fees in tokens 
4. Claim callable by anyone.

## Issue ticket number and link

## Checklist before requesting a review
- [x] I have performed a self-review of my code.
- [ ] If it is a core feature, I have added thorough tests.
- [x] I removed all Clippy and Formatting Warnings. 
- [x] I added required Copyrights.
  • Loading branch information
@Gauthamastro
Gauthamastro authored Nov 13, 2023
2 parents 5f0a92f + 3d790b1 commit bfa3582
Show file tree
Hide file tree
Showing 18 changed files with 375 additions and 191 deletions.
4 changes: 4 additions & 0 deletions Cargo.lock
View file

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

1 change: 0 additions & 1 deletion check-all-ci-tests.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,3 @@ cargo build --features runtime-benchmarks || exit
./target/debug/polkadex-node benchmark pallet --pallet "*" --extrinsic "*" --steps 2 --repeat 1 || exit
cargo clippy -- -D warnings || exit
cargo test || exit
RUSTFLAGS="-D warnings" cargo build -p thea-message-handler || exit
37 changes: 13 additions & 24 deletions pallets/ocex/src/benchmarking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@

use super::*;
use crate::Pallet as Ocex;
use frame_benchmarking::v1::{account, benchmarks};
use frame_benchmarking::{
v1::{account, benchmarks},
whitelisted_caller,
};
use frame_support::{
traits::{EnsureOrigin, UnfilteredDispatchable},
BoundedVec,
Expand Down Expand Up @@ -68,20 +71,11 @@ fn tpc(base_asset: AssetId, quote_asset: AssetId) -> TradingPairConfig {

benchmarks! {
register_main_account {
let b in 0 .. 50_000;
let origin = T::EnclaveOrigin::try_successful_origin().unwrap();
let account = T::EnclaveOrigin::try_successful_origin().unwrap();
let main: T::AccountId = match unsafe { origin.clone().into().unwrap_unchecked() } {
RawOrigin::Signed(account) => account.into(),
_ => panic!("wrong RawOrigin returned")
};
let proxy: T::AccountId = match unsafe { account.into().unwrap_unchecked() } {
RawOrigin::Signed(account) => account.into(),
_ => panic!("wrong RawOrigin returned")
};
let b in 0 .. 255;
let main: T::AccountId = whitelisted_caller();
let proxy = T::AccountId::decode(&mut &[b as u8; 32].to_vec()[..]).unwrap();
<ExchangeState<T>>::put(true);
let call = Call::<T>::register_main_account { proxy: proxy.clone() };
}: { call.dispatch_bypass_filter(origin)? }
}: _(RawOrigin::Signed(main.clone()), proxy.clone())
verify {
assert_last_event::<T>(Event::MainAccountRegistered {
main,
Expand All @@ -91,18 +85,13 @@ benchmarks! {

add_proxy_account {
let x in 0 .. 255; // should not overflow u8
let origin = T::EnclaveOrigin::try_successful_origin().unwrap();
let main: T::AccountId = match unsafe { origin.clone().into().unwrap_unchecked() } {
RawOrigin::Signed(account) => account.into(),
_ => panic!("wrong RawOrigin returned")
};
let main: T::AccountId = whitelisted_caller();
let proxy = T::AccountId::decode(&mut &[x as u8; 32].to_vec()[..]).unwrap();
<ExchangeState<T>>::put(true);
Ocex::<T>::register_main_account(origin.clone(), main.clone())?;
let call = Call::<T>::add_proxy_account { proxy: proxy.clone() };
}: { call.dispatch_bypass_filter(origin)? }
Ocex::<T>::register_main_account(RawOrigin::Signed(main.clone()).into(), main.clone())?;
}: _(RawOrigin::Signed(main.clone()), proxy.clone())
verify {
assert_last_event::<T>(Event::MainAccountRegistered {
assert_last_event::<T>(Event::NewProxyAdded {
main,
proxy
}.into());
Expand Down Expand Up @@ -366,7 +355,7 @@ benchmarks! {
BalanceOf::<T>::decode(&mut &(u128::MAX).to_le_bytes()[..]).unwrap()
)?;
let call = Call::<T>::claim_withdraw { snapshot_id: x as u64, account: main.clone() };
}: { call.dispatch_bypass_filter(origin)? }
}: _(RawOrigin::Signed(main.clone()), x as u64, main.clone())
verify {
assert_last_event::<T>(Event::WithdrawalClaimed {
main,
Expand Down
74 changes: 17 additions & 57 deletions pallets/ocex/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ pub mod pallet {
TradingPairIsNotClosed,
MainAccountAlreadyRegistered,
SnapshotNonceError,
/// Proxy is already in use
ProxyAlreadyRegistered,
EnclaveSignatureVerificationFailed,
MainAccountNotFound,
ProxyLimitExceeded,
Expand Down Expand Up @@ -333,60 +335,6 @@ pub mod pallet {

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
/// On idle, use the remaining weight to withdraw finalization
/// Automated (but much delayed) `claim_withdraw()` extrinsic
// fn on_idle(_n: BlockNumberFor<T>, mut remaining_weight: Weight) -> Weight {
// let snapshot_id = <SnapshotNonce<T>>::get();
// while remaining_weight.ref_time() >
// <T as Config>::WeightInfo::claim_withdraw(1).ref_time()
// {
// <Withdrawals<T>>::mutate(snapshot_id, |btree_map| {
// // Get mutable reference to the withdrawals vector
// if let Some(account) = btree_map.clone().keys().nth(1) {
// let mut accounts_to_clean = vec![];
// if let Some(withdrawal_vector) = btree_map.get_mut(account) {
// if let Some(withdrawal) = withdrawal_vector.pop() {
// if !Self::on_idle_withdrawal_processor(withdrawal.clone()) {
// withdrawal_vector.push(withdrawal.clone());
// Self::deposit_event(Event::WithdrawalFailed(withdrawal));
// }else{
// // TODO: enable this only after testing
// // Update events on successful withdrawal
// // let processed_withdrawls =
// // // Deposit event about successful withdraw
// // Self::deposit_event(Event::WithdrawalClaimed {
// // main: account.clone(),
// // withdrawals: processed_withdrawals.clone(),
// // });
// // <OnChainEvents<T>>::mutate(|onchain_events| {
// // onchain_events.push(
// // polkadex_primitives::ocex::OnChainEvents::OrderBookWithdrawalClaimed(
// // snapshot_id,
// // account.clone(),
// // processed_withdrawals,
// // ),
// // );
// // });
// }
// } else {
// // this user has no withdrawals left - remove from map
// accounts_to_clean.push(account.clone());
// }
// }
// for user in accounts_to_clean {
// btree_map.remove(&user);
// }
// }
// // we drain weight ALWAYS
// remaining_weight = remaining_weight
// .saturating_sub(<T as Config>::WeightInfo::claim_withdraw(1));
// });
// }
// remaining_weight
// }
/// What to do at the end of each block.
///
/// Clean OnCHainEvents
fn on_initialize(_n: BlockNumberFor<T>) -> Weight {
let len = <OnChainEvents<T>>::get().len();
if len > 0 {
Expand Down Expand Up @@ -437,8 +385,9 @@ pub mod pallet {
pub fn add_proxy_account(origin: OriginFor<T>, proxy: T::AccountId) -> DispatchResult {
let main_account = ensure_signed(origin)?;
ensure!(Self::orderbook_operational_state(), Error::<T>::ExchangeNotOperational);
// TODO: Avoid duplicate Proxy accounts
ensure!(<Accounts<T>>::contains_key(&main_account), Error::<T>::MainAccountNotFound);
// Avoid duplicate Proxy accounts
ensure!(!<Proxies<T>>::contains_key(&proxy), Error::<T>::ProxyAlreadyRegistered);
if let Some(mut account_info) = <Accounts<T>>::get(&main_account) {
ensure!(
account_info.add_proxy(proxy.clone()).is_ok(),
Expand All @@ -452,7 +401,8 @@ pub mod pallet {
));
});
<Accounts<T>>::insert(&main_account, account_info);
Self::deposit_event(Event::MainAccountRegistered { main: main_account, proxy });
<Proxies<T>>::insert(&proxy, main_account.clone());
Self::deposit_event(Event::NewProxyAdded { main: main_account, proxy });
}
Ok(())
}
Expand Down Expand Up @@ -820,8 +770,9 @@ pub mod pallet {
),
);
});
<Proxies<T>>::remove(proxy.clone());
Self::deposit_event(Event::ProxyRemoved { main: main_account.clone(), proxy });
}
Self::deposit_event(Event::ProxyRemoved { main: main_account.clone(), proxy });
Ok(())
})
}
Expand Down Expand Up @@ -1185,6 +1136,8 @@ pub mod pallet {
!<Accounts<T>>::contains_key(&main_account),
Error::<T>::MainAccountAlreadyRegistered
);
// Avoid duplicate Proxy accounts
ensure!(!<Proxies<T>>::contains_key(&proxy), Error::<T>::ProxyAlreadyRegistered);

let mut account_info = AccountInfo::new(main_account.clone());
ensure!(account_info.add_proxy(proxy.clone()).is_ok(), Error::<T>::ProxyLimitExceeded);
Expand All @@ -1197,6 +1150,7 @@ pub mod pallet {
proxy.clone(),
));
});
<Proxies<T>>::insert(&proxy, main_account.clone());
Self::deposit_event(Event::MainAccountRegistered { main: main_account, proxy });
Ok(())
}
Expand Down Expand Up @@ -1456,6 +1410,12 @@ pub mod pallet {
OptionQuery,
>;

// Proxy to main account map
#[pallet::storage]
#[pallet::getter(fn proxies)]
pub(super) type Proxies<T: Config> =
StorageMap<_, Blake2_128Concat, T::AccountId, T::AccountId, OptionQuery>;

// Trading pairs registered as Base, Quote => TradingPairInfo
#[pallet::storage]
#[pallet::getter(fn trading_pairs)]
Expand Down
30 changes: 17 additions & 13 deletions pallets/ocex/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,9 @@ fn test_add_proxy_account_exchange_state_not_operational() {
#[test]
fn test_add_proxy_account_proxy_limit_exceeded() {
let account_id = create_account_id();
let proxy_account = create_proxy_account();
let proxy_account1 = create_proxy_account("1");
let proxy_account2 = create_proxy_account("2");
let proxy_account3 = create_proxy_account("3");
new_test_ext().execute_with(|| {
assert_ok!(OCEX::set_exchange_state(RuntimeOrigin::root(), true));
assert_ok!(OCEX::register_main_account(
Expand All @@ -757,16 +759,16 @@ fn test_add_proxy_account_proxy_limit_exceeded() {
));
assert_ok!(OCEX::add_proxy_account(
RuntimeOrigin::signed(account_id.clone().into()),
account_id.clone().into()
proxy_account1.clone().into()
));
assert_ok!(OCEX::add_proxy_account(
RuntimeOrigin::signed(account_id.clone().into()),
account_id.clone().into()
proxy_account2.clone().into()
));
assert_noop!(
OCEX::add_proxy_account(
RuntimeOrigin::signed(account_id.clone().into()),
proxy_account.clone().into()
proxy_account3.clone().into()
),
Error::<Test>::ProxyLimitExceeded
);
Expand Down Expand Up @@ -799,21 +801,23 @@ fn test_add_proxy_account() {
RuntimeOrigin::signed(account_id.clone().into()),
account_id.clone().into()
));
assert_ok!(OCEX::add_proxy_account(
RuntimeOrigin::signed(account_id.clone().into()),
account_id.clone().into()
));
assert_noop!(
OCEX::add_proxy_account(
RuntimeOrigin::signed(account_id.clone().into()),
account_id.clone().into()
),
Error::<Test>::ProxyAlreadyRegistered
);
assert_last_event::<Test>(
crate::Event::MainAccountRegistered {
main: account_id.clone(),
proxy: account_id.clone(),
}
.into(),
);
let event: IngressMessages<AccountId32> =
IngressMessages::AddProxy(account_id.clone(), account_id.clone());

let blk = frame_system::Pallet::<Test>::current_block_number();
assert_eq!(OCEX::ingress_messages(blk)[2], event);
assert_eq!(OCEX::ingress_messages(blk).len(), 2);
});
}

Expand Down Expand Up @@ -2617,14 +2621,14 @@ fn create_account_id() -> AccountId32 {
return account_id
}

fn create_proxy_account() -> AccountId32 {
fn create_proxy_account(path: &str) -> AccountId32 {
const PHRASE: &str =
"news slush supreme milk chapter athlete soap sausage put clutch what kitten";
let keystore = MemoryKeystore::new();
let account_id: AccountId32 = <(dyn Keystore + 'static)>::sr25519_generate_new(
&keystore,
KEY_TYPE,
Some(&format!("{}/hunter2", PHRASE)),
Some(&format!("{}/{}", PHRASE, path)),
)
.expect("Unable to create sr25519 key pair")
.try_into()
Expand Down
2 changes: 2 additions & 0 deletions pallets/thea-executor/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ sp-std = { workspace = true, default-features = false }
polkadex-primitives = { workspace = true, default-features = false }
rust_decimal = { workspace = true, features = ["scale-codec"], default-features = false }
pallet-timestamp = { workspace = true, default-features = false }
pallet-asset-conversion = { workspace = true, default-features = false }
frame-benchmarking = { workspace = true, default-features = false, optional = true }
sp-core = { workspace = true, default-features = false }
sp-io = { workspace = true, default-features = false }
Expand All @@ -32,6 +33,7 @@ default = ["std"]
std = [
"log/std",
"sp-application-crypto/std",
"pallet-asset-conversion/std",
"thea-primitives/std",
"parity-scale-codec/std",
"scale-info/std",
Expand Down
Loading

0 comments on commit bfa3582

Please sign in to comment.