Skip to content

Commit df41033

Browse files
sacherjjsacherjj
authored
Merge pull request #5367 from darthsiroftardis/add-bid-limit-check
[BUGFIX] Fix incorrect setting of delegator min max limits on validator bids
2 parents c0cc9ce + a0aa92d commit df41033

File tree

12 files changed

+252
-42
lines changed

12 files changed

+252
-42
lines changed

‎execution_engine/src/runtime/mod.rs‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,23 +1066,15 @@ where
10661066
let minimum_delegation_amount = Self::try_get_named_argument(
10671067
runtime_args,
10681068
auction::ARG_MINIMUM_DELEGATION_AMOUNT,
1069-
)?
1070-
.unwrap_or(global_minimum_delegation_amount);
1069+
)?;
10711070

10721071
let global_maximum_delegation_amount =
10731072
self.context.engine_config().maximum_delegation_amount();
10741073
let maximum_delegation_amount = Self::try_get_named_argument(
10751074
runtime_args,
10761075
auction::ARG_MAXIMUM_DELEGATION_AMOUNT,
1077-
)?
1078-
.unwrap_or(global_maximum_delegation_amount);
1076+
)?;
10791077

1080-
if minimum_delegation_amount < global_minimum_delegation_amount
1081-
|| maximum_delegation_amount > global_maximum_delegation_amount
1082-
|| minimum_delegation_amount > maximum_delegation_amount
1083-
{
1084-
return Err(ExecError::Revert(ApiError::InvalidDelegationAmountLimits));
1085-
}
10861078
let reserved_slots =
10871079
Self::try_get_named_argument(runtime_args, auction::ARG_RESERVED_SLOTS)?
10881080
.unwrap_or(0);
@@ -1102,6 +1094,8 @@ where
11021094
minimum_bid_amount,
11031095
max_delegators_per_validator,
11041096
reserved_slots,
1097+
global_minimum_delegation_amount,
1098+
global_maximum_delegation_amount,
11051099
)
11061100
.map_err(Self::reverter)?;
11071101

‎execution_engine_testing/test_support/src/transfer_request_builder.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ impl TransferRequestBuilder {
5555
0,
5656
500_000_000_000,
5757
500_000_000_000,
58+
1_000_000_000_000_000_000,
5859
DEFAULT_GAS_HOLD_INTERVAL.millis(),
5960
false,
6061
Ratio::new_raw(U512::zero(), U512::zero()),
@@ -196,7 +197,12 @@ impl TransferRequestBuilder {
196197
.to_bytes()
197198
.unwrap(),
198199
);
199-
hasher.update(self.config.minimum_delegation_amount().to_bytes().unwrap());
200+
hasher.update(
201+
self.config
202+
.global_minimum_delegation_amount()
203+
.to_bytes()
204+
.unwrap(),
205+
);
200206
hasher.update(self.state_hash);
201207
hasher.update(self.block_time.to_bytes().unwrap());
202208
hasher.update(self.protocol_version.to_bytes().unwrap());

‎execution_engine_testing/test_support/src/wasm_test_builder.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,7 @@ where
824824
let max_delegators_per_validator = config.core_config.max_delegators_per_validator;
825825
let minimum_bid_amount = config.core_config.minimum_bid_amount;
826826
let minimum_delegation_amount = config.core_config.minimum_delegation_amount;
827+
let maximum_delegation_amount = config.core_config.maximum_delegation_amount;
827828
let balance_hold_interval = config.core_config.gas_hold_interval.millis();
828829
let include_credits = config.core_config.fee_handling == FeeHandling::NoFee;
829830
let credit_cap = Ratio::new_raw(
@@ -841,6 +842,7 @@ where
841842
max_delegators_per_validator,
842843
minimum_bid_amount,
843844
minimum_delegation_amount,
845+
maximum_delegation_amount,
844846
balance_hold_interval,
845847
include_credits,
846848
credit_cap,
@@ -1010,6 +1012,7 @@ where
10101012
self.chainspec.core_config.max_delegators_per_validator,
10111013
self.chainspec.core_config.minimum_bid_amount,
10121014
self.chainspec.core_config.minimum_delegation_amount,
1015+
self.chainspec.core_config.maximum_delegation_amount,
10131016
self.chainspec.core_config.gas_hold_interval.millis(),
10141017
include_credits,
10151018
credit_cap,

‎execution_engine_testing/tests/src/test/system_contracts/auction/bids.rs‎

Lines changed: 134 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use casper_execution_engine::{
2121
engine_state::{engine_config::DEFAULT_MINIMUM_DELEGATION_AMOUNT, Error},
2222
execution::ExecError,
2323
};
24-
use casper_storage::data_access_layer::{GenesisRequest, HandleFeeMode};
24+
use casper_storage::data_access_layer::{AuctionMethod, GenesisRequest, HandleFeeMode};
2525

2626
use crate::lmdb_fixture;
2727
use casper_types::{
@@ -836,8 +836,8 @@ fn should_forcibly_undelegate_after_setting_validator_limits() {
836836
ARG_PUBLIC_KEY => NON_FOUNDER_VALIDATOR_1_PK.clone(),
837837
ARG_AMOUNT => U512::from(1_000),
838838
ARG_DELEGATION_RATE => ADD_BID_DELEGATION_RATE_1,
839-
ARG_MINIMUM_DELEGATION_AMOUNT => DELEGATE_AMOUNT_2 + 1_000,
840-
ARG_MAXIMUM_DELEGATION_AMOUNT => DELEGATE_AMOUNT_1 - 1_000,
839+
ARG_MINIMUM_DELEGATION_AMOUNT => DELEGATE_AMOUNT_2 + 1_000, // 100
840+
ARG_MAXIMUM_DELEGATION_AMOUNT => DELEGATE_AMOUNT_1 - 1_000, // 1000
841841
},
842842
)
843843
.build();
@@ -5961,3 +5961,134 @@ fn should_mark_bids_with_less_than_minimum_bid_amount_as_inactive_via_upgrade()
59615961

59625962
assert!(bid.inactive())
59635963
}
5964+
5965+
#[ignore]
5966+
#[test]
5967+
fn should_correctly_allow_validator_to_change_delegator_min_max_limits() {
5968+
let validator_1_fund_request = ExecuteRequestBuilder::standard(
5969+
*DEFAULT_ACCOUNT_ADDR,
5970+
CONTRACT_TRANSFER_TO_ACCOUNT,
5971+
runtime_args! {
5972+
ARG_TARGET => *NON_FOUNDER_VALIDATOR_1_ADDR,
5973+
ARG_AMOUNT => U512::from(TRANSFER_AMOUNT)
5974+
},
5975+
)
5976+
.build();
5977+
5978+
let mut builder = LmdbWasmTestBuilder::default();
5979+
5980+
builder.run_genesis(LOCAL_GENESIS_REQUEST.clone());
5981+
5982+
builder
5983+
.exec(validator_1_fund_request)
5984+
.expect_success()
5985+
.commit();
5986+
5987+
let result = builder.bidding(
5988+
None,
5989+
DEFAULT_PROTOCOL_VERSION,
5990+
(*NON_FOUNDER_VALIDATOR_1_ADDR).into(),
5991+
AuctionMethod::AddBid {
5992+
public_key: NON_FOUNDER_VALIDATOR_1_PK.clone(),
5993+
delegation_rate: 10,
5994+
amount: U512::from(ADD_BID_AMOUNT_1),
5995+
minimum_delegation_amount: Some(DEFAULT_MINIMUM_DELEGATION_AMOUNT + 10),
5996+
maximum_delegation_amount: Some(DEFAULT_MAXIMUM_DELEGATION_AMOUNT - 10),
5997+
minimum_bid_amount: DEFAULT_MINIMUM_BID_AMOUNT,
5998+
reserved_slots: 0,
5999+
},
6000+
);
6001+
6002+
assert!(result.is_success());
6003+
builder.commit_transforms(builder.get_post_state_hash(), result.effects());
6004+
let validator_public_key = NON_FOUNDER_VALIDATOR_1_PK.clone();
6005+
6006+
let bid = builder
6007+
.get_bids()
6008+
.into_iter()
6009+
.find(|bid| bid.validator_public_key() == validator_public_key)
6010+
.expect("must have bid for the validator")
6011+
.as_validator_bid()
6012+
.expect("must get validator bid");
6013+
6014+
assert_eq!(
6015+
bid.minimum_delegation_amount(),
6016+
DEFAULT_MINIMUM_DELEGATION_AMOUNT + 10
6017+
);
6018+
6019+
let result = builder.bidding(
6020+
None,
6021+
DEFAULT_PROTOCOL_VERSION,
6022+
(*NON_FOUNDER_VALIDATOR_1_ADDR).into(),
6023+
AuctionMethod::AddBid {
6024+
public_key: NON_FOUNDER_VALIDATOR_1_PK.clone(),
6025+
delegation_rate: 10,
6026+
amount: U512::from(ADD_BID_AMOUNT_1),
6027+
minimum_delegation_amount: Some(DEFAULT_MINIMUM_DELEGATION_AMOUNT + 5),
6028+
maximum_delegation_amount: None,
6029+
minimum_bid_amount: DEFAULT_MINIMUM_BID_AMOUNT,
6030+
reserved_slots: 0,
6031+
},
6032+
);
6033+
6034+
assert!(result.is_success());
6035+
builder.commit_transforms(builder.get_post_state_hash(), result.effects());
6036+
6037+
let validator_public_key = NON_FOUNDER_VALIDATOR_1_PK.clone();
6038+
6039+
let bid = builder
6040+
.get_bids()
6041+
.into_iter()
6042+
.find(|bid| bid.validator_public_key() == validator_public_key)
6043+
.expect("must have bid for the validator")
6044+
.as_validator_bid()
6045+
.expect("must get validator bid");
6046+
6047+
assert_eq!(
6048+
bid.minimum_delegation_amount(),
6049+
DEFAULT_MINIMUM_DELEGATION_AMOUNT + 5
6050+
);
6051+
6052+
assert_eq!(
6053+
bid.maximum_delegation_amount(),
6054+
DEFAULT_MAXIMUM_DELEGATION_AMOUNT - 10
6055+
);
6056+
6057+
let result = builder.bidding(
6058+
None,
6059+
DEFAULT_PROTOCOL_VERSION,
6060+
(*NON_FOUNDER_VALIDATOR_1_ADDR).into(),
6061+
AuctionMethod::AddBid {
6062+
public_key: NON_FOUNDER_VALIDATOR_1_PK.clone(),
6063+
delegation_rate: 10,
6064+
amount: U512::from(ADD_BID_AMOUNT_1),
6065+
minimum_delegation_amount: None,
6066+
maximum_delegation_amount: Some(DEFAULT_MAXIMUM_DELEGATION_AMOUNT - 5),
6067+
minimum_bid_amount: DEFAULT_MINIMUM_BID_AMOUNT,
6068+
reserved_slots: 0,
6069+
},
6070+
);
6071+
6072+
assert!(result.is_success());
6073+
builder.commit_transforms(builder.get_post_state_hash(), result.effects());
6074+
6075+
let validator_public_key = NON_FOUNDER_VALIDATOR_1_PK.clone();
6076+
6077+
let bid = builder
6078+
.get_bids()
6079+
.into_iter()
6080+
.find(|bid| bid.validator_public_key() == validator_public_key)
6081+
.expect("must have bid for the validator")
6082+
.as_validator_bid()
6083+
.expect("must get validator bid");
6084+
6085+
assert_eq!(
6086+
bid.minimum_delegation_amount(),
6087+
DEFAULT_MINIMUM_DELEGATION_AMOUNT + 5
6088+
);
6089+
6090+
assert_eq!(
6091+
bid.maximum_delegation_amount(),
6092+
DEFAULT_MAXIMUM_DELEGATION_AMOUNT - 5
6093+
);
6094+
}

‎execution_engine_testing/tests/src/test/system_contracts/auction/distribute.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -998,8 +998,8 @@ fn should_distribute_rewards_after_restaking_delegated_funds() {
998998
public_key: VALIDATOR_1.clone(),
999999
amount,
10001000
delegation_rate: 0,
1001-
minimum_delegation_amount: undelegate_amount.as_u64(),
1002-
maximum_delegation_amount: undelegate_amount.as_u64(),
1001+
minimum_delegation_amount: Some(undelegate_amount.as_u64()),
1002+
maximum_delegation_amount: Some(undelegate_amount.as_u64()),
10031003
minimum_bid_amount: DEFAULT_MINIMUM_BID_AMOUNT,
10041004
reserved_slots: 0,
10051005
}

‎node/src/reactor/main_reactor/tests/fixture.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ impl TestFixture {
103103
.map(|(secret_key, stake)| {
104104
(
105105
PublicKey::from(secret_key.as_ref()),
106-
(U512::from(100_000_000_000_000_000u64), stake),
106+
(U512::from(700_000_000_000_000_000u64), stake),
107107
)
108108
})
109109
.collect();

‎storage/src/data_access_layer/auction.rs‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,11 @@ pub enum AuctionMethod {
5656
/// Bid amount.
5757
amount: U512,
5858
/// Minimum delegation amount for this validator bid.
59-
minimum_delegation_amount: u64,
59+
/// if provided by the user is set to Some
60+
minimum_delegation_amount: Option<u64>,
6061
/// Maximum delegation amount for this validator bid.
61-
maximum_delegation_amount: u64,
62+
/// if provided by the user is set to Some
63+
maximum_delegation_amount: Option<u64>,
6264
/// The minimum bid amount a validator must submit to have
6365
/// their bid considered as valid.
6466
minimum_bid_amount: u64,
@@ -145,12 +147,9 @@ impl AuctionMethod {
145147
Err(AuctionMethodError::InvalidEntryPoint(entry_point))
146148
}
147149
TransactionEntryPoint::ActivateBid => Self::new_activate_bid(runtime_args),
148-
TransactionEntryPoint::AddBid => Self::new_add_bid(
149-
runtime_args,
150-
chainspec.core_config.minimum_delegation_amount,
151-
chainspec.core_config.maximum_delegation_amount,
152-
chainspec.core_config.minimum_bid_amount,
153-
),
150+
TransactionEntryPoint::AddBid => {
151+
Self::new_add_bid(runtime_args, chainspec.core_config.minimum_bid_amount)
152+
}
154153
TransactionEntryPoint::WithdrawBid => {
155154
Self::new_withdraw_bid(runtime_args, chainspec.core_config.minimum_bid_amount)
156155
}
@@ -178,19 +177,15 @@ impl AuctionMethod {
178177

179178
fn new_add_bid(
180179
runtime_args: &RuntimeArgs,
181-
global_minimum_delegation: u64,
182-
global_maximum_delegation: u64,
183180
global_minimum_bid_amount: u64,
184181
) -> Result<Self, AuctionMethodError> {
185182
let public_key = Self::get_named_argument(runtime_args, auction::ARG_PUBLIC_KEY)?;
186183
let delegation_rate = Self::get_named_argument(runtime_args, auction::ARG_DELEGATION_RATE)?;
187184
let amount = Self::get_named_argument(runtime_args, auction::ARG_AMOUNT)?;
188185
let minimum_delegation_amount =
189-
Self::get_named_argument(runtime_args, auction::ARG_MINIMUM_DELEGATION_AMOUNT)
190-
.unwrap_or(global_minimum_delegation);
186+
Self::try_get_named_argument(runtime_args, auction::ARG_MINIMUM_DELEGATION_AMOUNT)?;
191187
let maximum_delegation_amount =
192-
Self::get_named_argument(runtime_args, auction::ARG_MAXIMUM_DELEGATION_AMOUNT)
193-
.unwrap_or(global_maximum_delegation);
188+
Self::try_get_named_argument(runtime_args, auction::ARG_MAXIMUM_DELEGATION_AMOUNT)?;
194189
let reserved_slots =
195190
Self::get_named_argument(runtime_args, auction::ARG_RESERVED_SLOTS).unwrap_or(0);
196191

@@ -329,6 +324,25 @@ impl AuctionMethod {
329324
error,
330325
})
331326
}
327+
328+
fn try_get_named_argument<T: FromBytes + CLTyped>(
329+
args: &RuntimeArgs,
330+
name: &str,
331+
) -> Result<Option<T>, AuctionMethodError> {
332+
match args.get(name) {
333+
Some(arg) => {
334+
let arg = arg
335+
.clone()
336+
.into_t()
337+
.map_err(|error| AuctionMethodError::CLValue {
338+
arg: name.to_string(),
339+
error,
340+
})?;
341+
Ok(Some(arg))
342+
}
343+
None => Ok(None),
344+
}
345+
}
332346
}
333347

334348
/// Bidding request.

‎storage/src/global_state/state/mod.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,6 +1225,9 @@ pub trait StateProvider: Send + Sync + Sized {
12251225
let address_generator = AddressGenerator::new(&id.seed(), phase);
12261226
let max_delegators_per_validator = config.max_delegators_per_validator();
12271227
let minimum_bid_amount = config.minimum_bid_amount();
1228+
1229+
let global_minimum_delegation_limit = config.global_minimum_delegation_amount();
1230+
let global_maximum_delegation_limit = config.global_maximum_delegation_amount();
12281231
let mut runtime = RuntimeNative::new(
12291232
config,
12301233
protocol_version,
@@ -1264,6 +1267,8 @@ pub trait StateProvider: Send + Sync + Sized {
12641267
minimum_bid_amount,
12651268
max_delegators_per_validator,
12661269
reserved_slots,
1270+
global_minimum_delegation_limit,
1271+
global_maximum_delegation_limit,
12671272
)
12681273
.map(AuctionMethodRet::UpdatedAmount)
12691274
.map_err(TrackingCopyError::Api),

0 commit comments

Comments
 (0)