Skip to content
This repository was archived by the owner on Sep 28, 2023. It is now read-only.
Draft
Show file tree
Hide file tree
Changes from 12 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
24 changes: 24 additions & 0 deletions chain-extensions/dapps-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ enum DappsStakingFunc {
ClaimDapp,
SetRewardDestination,
NominationTransfer,
RebondAndStake,
}

impl TryFrom<u32> for DappsStakingFunc {
Expand All @@ -58,6 +59,7 @@ impl TryFrom<u32> for DappsStakingFunc {
12 => Ok(DappsStakingFunc::ClaimDapp),
13 => Ok(DappsStakingFunc::SetRewardDestination),
14 => Ok(DappsStakingFunc::NominationTransfer),
15 => Ok(DappsStakingFunc::RebondAndStake),
_ => Err(DispatchError::Other(
"DappsStakingExtension: Unimplemented func_id",
)),
Expand Down Expand Up @@ -333,6 +335,28 @@ impl<T: pallet_dapps_staking::Config> ChainExtensionExec<T> for DappsStakingExte
Ok(_) => Ok(RetVal::Converging(DSError::Success as u32)),
};
}

DappsStakingFunc::RebondAndStake => {
let contract_bytes: [u8; 32] = env.read_as()?;
Comment thread
shunsukew marked this conversation as resolved.
Outdated
let contract = Self::decode_smart_contract(contract_bytes)?;

let base_weight =
<T as pallet_dapps_staking::Config>::WeightInfo::rebond_and_stake();
env.charge_weight(base_weight)?;

let caller = env.ext().address().clone();
let call_result = pallet_dapps_staking::Pallet::<T>::rebond_and_stake(
RawOrigin::Signed(caller).into(),
contract,
);
Comment on lines +346 to +354

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was kinda confused since rebond_and_stake is called twice in a row. I do understand that these are two different calls, but still this is a bit confusing. Maybe we should add couple of comments to reduce overall WTF/minute.

@shunsukew shunsukew Sep 28, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This can be said for other functions too. it's a more universal topic.
I thought it's very clear they are different and what they do because they are different methods, one is WeightInfo's and another is Pallet's.

return match call_result {
Err(e) => {
let mapped_error = DSError::try_from(e.error)?;
Ok(RetVal::Converging(mapped_error as u32))
}
Ok(_) => Ok(RetVal::Converging(DSError::Success as u32)),
};
}
}

Ok(RetVal::Converging(DSError::Success as u32))
Expand Down
3 changes: 3 additions & 0 deletions chain-extensions/types/dapps-staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ pub enum DSError {
NominationTransferToSameContract = 26,
/// Unexpected reward destination value
RewardDestinationValueOutOfBounds = 27,
/// There are no previously unbonded funds that can be reboneded and staked.
Comment thread
shunsukew marked this conversation as resolved.
Outdated
NothingToRebond = 28,
/// Unknown error
UnknownError = 99,
}
Expand Down Expand Up @@ -106,6 +108,7 @@ impl TryFrom<DispatchError> for DSError {
Some("NominationTransferToSameContract") => {
Ok(DSError::NominationTransferToSameContract)
}
Some("NothingToRebond") => Ok(DSError::NothingToRebond),
_ => Ok(DSError::UnknownError),
};
}
Expand Down
18 changes: 18 additions & 0 deletions frame/dapps-staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,24 @@ benchmarks! {
assert_last_event::<T>(Event::<T>::NominationTransfer(staker, origin_contract_id, T::MinimumStakingAmount::get(), target_contract_id).into());
}

rebond_and_stake {
initialize::<T>();

let (_, contract_id) = register_contract::<T>(1)?;
prepare_bond_and_stake::<T>(T::MaxNumberOfStakersPerContract::get() - 1, &contract_id, SEED)?;

let staker = whitelisted_caller();
let _ = T::Currency::make_free_balance_be(&staker, BalanceOf::<T>::max_value());
let stake_amount = BalanceOf::<T>::max_value() / 2u32.into();
let unstake_amount = stake_amount / 2u32.into();

DappsStaking::<T>::bond_and_stake(RawOrigin::Signed(staker.clone()).into(), contract_id.clone(), stake_amount)?;
DappsStaking::<T>::unbond_and_unstake(RawOrigin::Signed(staker.clone()).into(), contract_id.clone(), unstake_amount)?;
}: _(RawOrigin::Signed(staker.clone()), contract_id.clone())
verify {
assert_last_event::<T>(Event::<T>::RebondAndStake(staker, contract_id, unstake_amount).into());
}
Comment on lines +220 to +239

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I'd like to have some comments describing what is actually happening.


claim_staker_with_restake {
initialize::<T>();
let (_, contract_id) = register_contract::<T>(1)?;
Expand Down
62 changes: 62 additions & 0 deletions frame/dapps-staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ pub mod pallet {
BalanceOf<T>,
T::SmartContract,
),
/// Account has rebonded unlocking chunks and staked funds on a smart contract.
RebondAndStake(T::AccountId, T::SmartContract, BalanceOf<T>),
}

#[pallet::error]
Expand Down Expand Up @@ -270,6 +272,8 @@ pub mod pallet {
NotActiveStaker,
/// Transfering nomination to the same contract
NominationTransferToSameContract,
/// There are no previously unbonded funds that can be rebonded and re-staked.
NothingToRebond,
}

#[pallet::hooks]
Expand Down Expand Up @@ -570,6 +574,64 @@ pub mod pallet {
Ok(().into())
}

/// Lock up and stake unbonded chunks of origin account.
///
/// All unbonding chunks will be used and staked to the specified contract.
///
/// The dispatch origin for this call must be _Signed_ by the staker's account.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please also check top of lib.rs file and update comments there if needed.

#[pallet::weight(T::WeightInfo::rebond_and_stake())]
pub fn rebond_and_stake(
origin: OriginFor<T>,
contract_id: T::SmartContract,
) -> DispatchResultWithPostInfo {
Self::ensure_pallet_enabled()?;
let staker = ensure_signed(origin)?;

// Check that contract is ready for staking.
ensure!(
Self::is_active(&contract_id),
Error::<T>::NotOperatedContract
);

// Get the staking ledger or create an entry if it doesn't exist.
let mut ledger = Self::ledger(&staker);
let value_to_stake = ledger.unbonding_info.sum();
ensure!(value_to_stake > Zero::zero(), Error::<T>::NothingToRebond);

let current_era = Self::current_era();
let mut staking_info =
Self::contract_stake_info(&contract_id, current_era).unwrap_or_default();
let mut staker_info = Self::staker_info(&staker, &contract_id);

Self::stake_on_contract(
&mut staker_info,
&mut staking_info,
value_to_stake,
current_era,
)?;

ledger.unbonding_info.unlocking_chunks = Vec::<UnlockingChunk<BalanceOf<T>>>::default();

GeneralEraInfo::<T>::mutate(&current_era, |value| {
if let Some(x) = value {
x.staked = x.staked.saturating_add(value_to_stake);
x.locked = x.locked.saturating_add(value_to_stake);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is incorrect - TVL doesn't increase after this since unbonding chunks are still considered to be locked.

UT should be updated to catch this.

}
});

Self::update_ledger(&staker, ledger);
Self::update_staker_info(&staker, &contract_id, staker_info);
ContractEraStake::<T>::insert(&contract_id, current_era, staking_info);

Self::deposit_event(Event::<T>::RebondAndStake(
staker,
contract_id,
value_to_stake,
));

Ok(().into())
}

/// Withdraw all funds that have completed the unbonding process.
///
/// If there are unbonding chunks which will be fully unbonded in future eras,
Expand Down
61 changes: 61 additions & 0 deletions frame/dapps-staking/src/testing_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,67 @@ pub(crate) fn assert_unbond_and_unstake(
assert_eq!(init_state.era_info.locked, final_state.era_info.locked);
}

pub(crate) fn assert_rebond_and_stake(
staker: AccountId,
contract_id: &MockSmartContract<AccountId>,
) {
// Get latest staking info
let current_era = DappsStaking::current_era();
let init_state = MemorySnapshot::all(current_era, &contract_id, staker);

// Define expected stake amount
let expected_stake_amount = init_state.ledger.unbonding_info.sum();

// Ensure op is successful and event is emitted
assert_ok!(DappsStaking::rebond_and_stake(
Origin::signed(staker),
contract_id.clone(),
));
System::assert_last_event(mock::Event::DappsStaking(Event::RebondAndStake(
staker,
contract_id.clone(),
expected_stake_amount,
)));

// Fetch the latest unbonding info so we can compare it to initial unbonding info
let final_state = MemorySnapshot::all(current_era, &contract_id, staker);
assert!(final_state.ledger.unbonding_info.is_empty());

// locked amount before and after the operation should be the same.
// unlocking chunks are still locked unless it is withdrawn.
assert_eq!(final_state.ledger.locked, init_state.ledger.locked);

// In case staker hasn't been staking this contract until now
if init_state.staker_info.latest_staked_value() == 0 {
assert!(GeneralStakerInfo::<TestRuntime>::contains_key(
&staker,
contract_id
));
assert_eq!(
final_state.contract_info.number_of_stakers,
init_state.contract_info.number_of_stakers + 1
);
}

// Verify the remaining states
assert_eq!(
final_state.era_info.staked,
init_state.era_info.staked + expected_stake_amount
);
assert_eq!(
final_state.era_info.locked,
init_state.era_info.locked + expected_stake_amount
);
Comment on lines +403 to +406

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is incorrect, see my comment above.

assert_eq!(
final_state.contract_info.total,
init_state.contract_info.total + expected_stake_amount
);
assert_eq!(
final_state.staker_info.latest_staked_value(),
init_state.staker_info.latest_staked_value() + expected_stake_amount
);
}

/// Used to perform start_unbonding with success and storage assertions.
pub(crate) fn assert_withdraw_unbonded(staker: AccountId) {
let current_era = DappsStaking::current_era();
Expand Down
65 changes: 65 additions & 0 deletions frame/dapps-staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,6 +1059,71 @@ fn unbond_and_unstake_with_no_chunks_allowed() {
})
}

#[test]
fn rebond_and_stake_is_ok() {
ExternalityBuilder::build().execute_with(|| {
initialize_first_block();

let contract_id = MockSmartContract::Evm(H160::repeat_byte(0x01));
let rebond_contract_id = MockSmartContract::Evm(H160::repeat_byte(0x02));
assert_register(10, &contract_id);
assert_register(11, &rebond_contract_id);

let staker_id = 1;
assert_bond_and_stake(staker_id, &contract_id, 1000);

let first_unbond_value = 100;
let second_unbond_value = 250;
let initial_era = DappsStaking::current_era();

// Unbond some amount in the initial era
assert_unbond_and_unstake(staker_id, &contract_id, first_unbond_value);

// Advance one era and then unbond some more
advance_to_era(initial_era + 1);
assert_unbond_and_unstake(staker_id, &contract_id, second_unbond_value);

// rebond and stake
assert_rebond_and_stake(staker_id, &rebond_contract_id);
})
}

#[test]
fn rebond_and_stake_unexist_contract_fails() {
ExternalityBuilder::build().execute_with(|| {
initialize_first_block();

let staker_id = 1;
let contract_id = MockSmartContract::Evm(H160::repeat_byte(0x01));
assert_register(10, &contract_id);

assert_bond_and_stake(staker_id, &contract_id, 1000);
assert_unbond_and_unstake(staker_id, &contract_id, 100);

let non_exist_contract_id = MockSmartContract::Evm(H160::repeat_byte(0x02));
assert_noop!(
DappsStaking::rebond_and_stake(Origin::signed(staker_id), non_exist_contract_id),
Error::<TestRuntime>::NotOperatedContract,
);
})
}

#[test]
fn rebond_and_stake_no_unbonding_chunks_fails() {
ExternalityBuilder::build().execute_with(|| {
initialize_first_block();

let staker_id = 1;
let contract_id = MockSmartContract::Evm(H160::repeat_byte(0x01));
assert_register(10, &contract_id);

assert_noop!(
DappsStaking::rebond_and_stake(Origin::signed(staker_id), contract_id),
Error::<TestRuntime>::NothingToRebond,
);
})
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'd suggest also covering the error cases with InsufficientValue or TooManyEraStakeValues.

Not mandatory, just a suggestion.

#[test]
fn withdraw_unbonded_is_ok() {
ExternalityBuilder::build().execute_with(|| {
Expand Down
27 changes: 27 additions & 0 deletions frame/dapps-staking/src/weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ pub trait WeightInfo {
fn developer_pre_approval() -> Weight;
fn bond_and_stake() -> Weight;
fn unbond_and_unstake() -> Weight;
fn rebond_and_stake() -> Weight;
fn withdraw_unbonded() -> Weight;
fn claim_staker_without_restake() -> Weight;
fn claim_staker_with_restake() -> Weight;
Expand Down Expand Up @@ -98,6 +99,19 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
.saturating_add(T::DbWeight::get().writes(5 as Weight))
}
// Storage: DappsStaking PalletDisabled (r:1 w:0)
// Storage: DappsStaking RegisteredDapps (r:1 w:0)
// Storage: DappsStaking Ledger (r:1 w:1)
// Storage: DappsStaking CurrentEra (r:1 w:0)
// Storage: DappsStaking ContractEraStake (r:1 w:1)
// Storage: DappsStaking GeneralStakerInfo (r:1 w:1)
// Storage: DappsStaking GeneralEraInfo (r:1 w:1)
// Storage: Balances Locks (r:1 w:1)
fn rebond_and_stake() -> Weight {
(133_638_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(8 as Weight))
.saturating_add(RocksDbWeight::get().writes(5 as Weight))
}
// Storage: DappsStaking PalletDisabled (r:1 w:0)
// Storage: DappsStaking Ledger (r:1 w:1)
// Storage: DappsStaking CurrentEra (r:1 w:0)
// Storage: Balances Locks (r:1 w:1)
Expand Down Expand Up @@ -249,6 +263,19 @@ impl WeightInfo for () {
.saturating_add(RocksDbWeight::get().writes(5 as Weight))
}
// Storage: DappsStaking PalletDisabled (r:1 w:0)
// Storage: DappsStaking RegisteredDapps (r:1 w:0)
// Storage: DappsStaking Ledger (r:1 w:1)
// Storage: DappsStaking CurrentEra (r:1 w:0)
// Storage: DappsStaking ContractEraStake (r:1 w:1)
// Storage: DappsStaking GeneralStakerInfo (r:1 w:1)
// Storage: DappsStaking GeneralEraInfo (r:1 w:1)
// Storage: Balances Locks (r:1 w:1)
fn rebond_and_stake() -> Weight {
(133_638_000 as Weight)
.saturating_add(RocksDbWeight::get().reads(8 as Weight))
.saturating_add(RocksDbWeight::get().writes(5 as Weight))
}
// Storage: DappsStaking PalletDisabled (r:1 w:0)
// Storage: DappsStaking Ledger (r:1 w:1)
// Storage: DappsStaking CurrentEra (r:1 w:0)
// Storage: Balances Locks (r:1 w:1)
Expand Down
4 changes: 4 additions & 0 deletions precompiles/dapps-staking/DappsStaking.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,8 @@ interface DappsStaking {
/// @param amount: The amount to transfer from origin to target
/// @param target_smart_contract: The target smart contract address
function nomination_transfer(address origin_smart_contract, uint128 amount, address target_smart_contract) external;

/// @notice Rebond and stake all funds that are in the unbonding process.
/// @param smart_contract: The smart contract address to stake
function rebond_and_stake(address smart_contract) external;
}
Loading