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.

Sign up for GitHub

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

Jump to bottom

Fix UX Issues #880

Merged
merged 15 commits into from
Nov 13, 2023
Merged

Fix UX Issues #880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7e214ee
Add logic for swapping ED from polkapool
Gauthamastro Nov 3, 2023
87b98fd
Add logic to pay withdrawal fees in other tokens
Gauthamastro Nov 3, 2023
30b30c3
Avoid duplicate proxies
Gauthamastro Nov 3, 2023
2d69dc8
cargo and taplo fmt
Gauthamastro Nov 3, 2023
d538fa4
Remove on_idle and make claim_deposit ensure signed
Gauthamastro Nov 3, 2023
c43cda3
Fix benchmarks
Gauthamastro Nov 3, 2023
b0ee672
Fix tests
Gauthamastro Nov 3, 2023
bcbeb30
Refactor Swap integration
Gauthamastro Nov 4, 2023
ee3b6b2
Update tests
Gauthamastro Nov 4, 2023
c341eb4
Add transactional macro for claim deposit
Gauthamastro Nov 4, 2023
28e7a13
taplo fmt
Gauthamastro Nov 4, 2023
53ad646
Fix benchmarkign
Gauthamastro Nov 7, 2023
4fe65b5
Fix CI
Gauthamastro Nov 10, 2023
673fc05
Merge remote-tracking branch 'origin/gj/ux-fixes' into gj/ux-fixes
Gauthamastro Nov 10, 2023
3d790b1
Taplo fmt
Gauthamastro Nov 10, 2023
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
4 changes: 4 additions & 0 deletions Cargo.lock
View file
Open in desktop

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
Open in desktop
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
Open in desktop
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
Open in desktop
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);
Gauthamastro marked this conversation as resolved.
Show resolved Hide resolved
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
Open in desktop
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
Open in desktop
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
Loading