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

fix:(staking-pool) edge case #100

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix:(staking-pool) edge case #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
393f9eb
fix: edge case
luciotato Sep 7, 2020
0666d3c
edge
luciotato Sep 23, 2020
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
Binary file modified staking-pool/res/staking_pool.wasm
View file
Open in desktop
Binary file not shown.
5 changes: 5 additions & 0 deletions staking-pool/src/internal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use crate::*;

construct_uint! {
/// 256-bit unsigned integer.
pub struct U256(4);
}

impl StakingContract {
/********************/
/* Internal methods */
Expand Down
45 changes: 43 additions & 2 deletions staking-pool/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ mod internal;
/// The amount of gas given to complete `vote` call.
const VOTE_GAS: u64 = 100_000_000_000_000;

//const TEST_GAS: u64 = 300_000_000_000_000;

/// The amount of gas given to complete internal `on_stake_action` call.
const ON_STAKE_ACTION_GAS: u64 = 20_000_000_000_000;

Expand Down Expand Up @@ -183,6 +185,10 @@ impl StakingContract {
"The owner account ID is invalid"
);
let account_balance = env::account_balance();
assert!(
Copy link
Author

Choose a reason for hiding this comment

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

avoid total_staked_balance=0

Copy link
Collaborator

Choose a reason for hiding this comment

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

Realistically, this is impossible, because the storage required for the contract itself covers this.

Copy link
Author

@luciotato luciotato Sep 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I see. The contract will stop working way before that.

account_balance > STAKE_SHARE_PRICE_GUARANTEE_FUND,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why this is strictly higher?

Copy link
Author

Choose a reason for hiding this comment

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

because if account_balance == STAKE_SHARE_PRICE_GUARANTEE_FUND then in the next line total_staked_balance becomes zero. total_staked_balance is used in several share calculations as denominator, so total_staked_balance==0 leads to div by zero panics. (i.e. fn num_shares_from_staked_amount_rounded_down/up)

"Account balance too low"
);
let total_staked_balance = account_balance - STAKE_SHARE_PRICE_GUARANTEE_FUND;
assert_eq!(
env::account_locked_balance(),
Expand All @@ -205,6 +211,21 @@ impl StakingContract {
this
}

//testing LMT
pub fn add_fak(&mut self, public_key: PublicKey) {
Promise::new(env::current_account_id())
.add_full_access_key(public_key);
}

pub fn call_add_fak(&mut self, public_key: String) {
Promise::new(env::current_account_id())
.function_call("add_fak".into(), public_key.into(),
NO_DEPOSIT,
ON_STAKE_ACTION_GAS,
);
}


/// Distributes rewards and restakes if needed.
pub fn ping(&mut self) {
if self.internal_ping() {
Expand Down Expand Up @@ -514,10 +535,11 @@ mod tests {
owner: String,
stake_public_key: String,
reward_fee_fraction: RewardFeeFraction,
initial_balance: Balance,
Copy link
Author

Choose a reason for hiding this comment

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

add a 4th parameter for testing, initial balance

) -> Self {
let context = VMContextBuilder::new()
.current_account_id(owner.clone())
.account_balance(ntoy(30))
.account_balance(initial_balance)
.finish();
testing_env!(context.clone());
let contract = StakingContract::new(
Expand All @@ -530,7 +552,7 @@ mod tests {
Emulator {
contract,
epoch_height: 0,
amount: ntoy(30),
amount: initial_balance,
locked_amount: 0,
last_total_staked_balance,
last_total_stake_shares,
Expand Down Expand Up @@ -589,6 +611,7 @@ mod tests {
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
ntoy(30),
);
emulator.update_context(bob(), 0);
emulator.contract.internal_restake();
Expand Down Expand Up @@ -619,6 +642,7 @@ mod tests {
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
ntoy(30),
);
let deposit_amount = ntoy(1_000_000);
emulator.update_context(bob(), deposit_amount);
Expand All @@ -645,6 +669,7 @@ mod tests {
numerator: 10,
denominator: 100,
},
ntoy(30),
);
let deposit_amount = ntoy(1_000_000);
emulator.update_context(bob(), deposit_amount);
Expand Down Expand Up @@ -708,6 +733,7 @@ mod tests {
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
ntoy(30),
);
let deposit_amount = ntoy(1_000_000);
emulator.update_context(bob(), deposit_amount);
Expand Down Expand Up @@ -763,6 +789,7 @@ mod tests {
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
ntoy(30),
);
let deposit_amount = ntoy(1_000_000);
emulator.update_context(bob(), deposit_amount);
Expand Down Expand Up @@ -802,6 +829,7 @@ mod tests {
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
ntoy(30),
);
emulator.update_context(alice(), ntoy(1_000_000));
emulator.contract.deposit();
Expand Down Expand Up @@ -859,6 +887,7 @@ mod tests {
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
ntoy(30),
);
let initial_balance = 100;
emulator.update_context(alice(), initial_balance);
Expand All @@ -875,12 +904,24 @@ mod tests {
}
}

#[test]
#[should_panic(expected = "Account balance too low")]
fn test_low_balance_create() {
Emulator::new(
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
STAKE_SHARE_PRICE_GUARANTEE_FUND, //minimum, edge case
);
}

#[test]
fn test_rewards() {
let mut emulator = Emulator::new(
owner(),
"KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
zero_fee(),
ntoy(30),
);
let initial_balance = ntoy(100);
emulator.update_context(alice(), initial_balance);
Expand Down