added node side withdraw fn #690
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## eth-rollup-develop #690 +/- ##
======================================================
- Coverage 51.84% 51.84% -0.01%
======================================================
Files 44 44
Lines 7389 7390 +1
======================================================
Hits 3831 3831
- Misses 3558 3559 +1 ☔ View full report in Codecov by Sentry. |
| @@ -311,6 +311,50 @@ pub mod pallet { | |||
| Ok(().into()) | |||
| } | |||
|
|
|||
| #[pallet::call_index(3)] | |||
| #[pallet::weight(T::DbWeight::get().reads_writes(1, 1).saturating_add(Weight::from_parts(40_000_000, 0)))] | |||
| pub fn withdraw( | |||
There was a problem hiding this comment.
please add unit tests for the method that covers basic scenarios:
- check if total allocation of tokens has changed
- counter is updated
pallets/rolldown/src/lib.rs
Outdated
| ) -> DispatchResultWithPostInfo { | ||
| let account = ensure_signed(origin)?; | ||
|
|
||
| let eth_token_address: [u8; 20] = array_bytes::hex2array::<_, 20>(tokenAddress.clone()) |
There was a problem hiding this comment.
tjoken address is already of a type, convertion is not needed
pallets/rolldown/src/lib.rs
Outdated
| <T as Config>::Tokens::ensure_can_withdraw( | ||
| asset_id.into(), | ||
| &account, | ||
| amount.try_into().map_err(|_| "u128 to Balance failed")?, |
There was a problem hiding this comment.
please use regular substrate Errors and not string
pallets/rolldown/src/lib.rs
Outdated
| WithdrawReasons::all(), | ||
| Default::default(), | ||
| ) | ||
| .or(Err("NotEnoughAssets"))?; |
pallets/rolldown/src/lib.rs
Outdated
| T::Tokens::burn_and_settle( | ||
| asset_id, | ||
| &account, | ||
| amount.try_into().map_err(|_| "u128 to Balance failed")?, |
There was a problem hiding this comment.
please use dedicated error type
pallets/rolldown/src/lib.rs
Outdated
| )?; | ||
|
|
||
| // increase counter for updates originating on l2 | ||
| l2_origin_updates_counter::<T>::put(Self::get_l2_origin_updates_counter() + 1); |
There was a problem hiding this comment.
please use safe math instead
pallets/rolldown/src/lib.rs
Outdated
|
|
||
| Ok(()) | ||
| } | ||
| // fn process_withdraw(withdraw_request_details: &messages::Withdraw) -> Result<(), &'static str> { |
There was a problem hiding this comment.
why its commented out ? please either uncomment or delete. (its in git history either way)
pallets/rolldown/src/lib.rs
Outdated
| @@ -625,6 +675,7 @@ impl<T: Config> Pallet<T> { | |||
| // slash is after adding rights, since slash can reduce stake below required level and remove all rights | |||
| Self::slash(&to_be_slashed); | |||
|
|
|||
| log!(info, "Cancel resolutiuon processed successfully: {:?}", cancel_resolution); | |||
pallets/rolldown/src/messages.rs
Outdated
| Deposit[] pendingDeposits; | ||
| CancelResolution[] pendingCancelResultions; | ||
| L2UpdatesToRemove[] pendingL2UpdatesToRemove; | ||
| } | ||
|
|
||
|
|
||
| #[derive(Debug, Eq, PartialEq, Encode, Decode, TypeInfo)] | ||
| enum UpdateType{ DEPOSIT, WITHDRAWAL, INDEX_UPDATE, CANCEL_RESOLUTION} | ||
| enum UpdateType{ DEPOSIT, WITHDRAW, INDEX_UPDATE, CANCEL_RESOLUTION} |
pallets/rolldown/src/messages.rs
Outdated
| @@ -170,6 +156,11 @@ pub mod eth_abi { | |||
| bool status; | |||
| } | |||
|
|
|||
| struct Withdraw { | |||
| Error::<Test>::NotEnoughAssets | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
please add ut that makes sure that now with new request_id the tx will be removed from the list of pending updates after proper l1update is received
There was a problem hiding this comment.
i think it will be rejected because of too high request_id or sth
added node side withdraw fn