Skip to content

Commit

Permalink
Merge pull request #2984 from blockstack/feat/pessimistic-estimator-fix
Browse files Browse the repository at this point in the history
Feat: update pessimistic cost estimator to differentiate by contract sender
  • Loading branch information
kantai authored Jan 7, 2022
2 parents fd3f28d + 8750b69 commit 98cdf3d
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 2 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to the versioning scheme outlined in the [README.md](README.md).

## [Unreleased]

### Changed

- The pessimistic execution cost estimator differentiates between
contracts with different origin addresses.

## [2.05.0.0.0]

This software update is a consensus changing release and the
Expand Down
4 changes: 2 additions & 2 deletions src/cost_estimates/pessimistic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,8 @@ impl PessimisticEstimator {
StacksEpochId::Epoch2_05 => ":2.05",
};
format!(
"cc{}:{}.{}",
epoch_marker, cc.contract_name, cc.function_name
"cc{}:{}:{}.{}",
epoch_marker, cc.address, cc.contract_name, cc.function_name
)
}
TransactionPayload::SmartContract(_sc) => "contract-publish".to_string(),
Expand Down
98 changes: 98 additions & 0 deletions src/cost_estimates/tests/cost_estimators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,104 @@ fn test_pessimistic_cost_estimator_declining_average() {
);
}

#[test]
/// This tests the PessimisticEstimator as a unit (i.e., separate
/// from the trait auto-impl method) by providing payload inputs
/// to produce the expected pessimistic result (i.e., mean over a 10-sample
/// window, where the window only updates if the new entry would make a dimension
/// worse).
fn pessimistic_estimator_contract_owner_separation() {
let mut estimator = instantiate_test_db();
let cc_payload_0 = TransactionPayload::ContractCall(TransactionContractCall {
address: StacksAddress::new(0, Hash160([0; 20])),
contract_name: "contract-1".into(),
function_name: "func1".into(),
function_args: vec![],
});
let cc_payload_1 = TransactionPayload::ContractCall(TransactionContractCall {
address: StacksAddress::new(0, Hash160([1; 20])),
contract_name: "contract-1".into(),
function_name: "func1".into(),
function_args: vec![],
});

estimator
.notify_event(
&cc_payload_0,
&ExecutionCost {
write_length: 1,
write_count: 1,
read_length: 1,
read_count: 1,
runtime: 1,
},
&BLOCK_LIMIT_MAINNET_20,
&StacksEpochId::Epoch20,
)
.expect("Should be able to process event");

assert_eq!(
estimator.estimate_cost(&cc_payload_1, &StacksEpochId::Epoch20,),
Err(EstimatorError::NoEstimateAvailable)
);

assert_eq!(
estimator
.estimate_cost(&cc_payload_0, &StacksEpochId::Epoch20,)
.expect("Should be able to provide cost estimate now"),
ExecutionCost {
write_length: 1,
write_count: 1,
read_length: 1,
read_count: 1,
runtime: 1,
}
);

estimator
.notify_event(
&cc_payload_1,
&ExecutionCost {
write_length: 5,
write_count: 5,
read_length: 5,
read_count: 5,
runtime: 5,
},
&BLOCK_LIMIT_MAINNET_20,
&StacksEpochId::Epoch20,
)
.expect("Should be able to process event");

// cc_payload_0 should not be affected
assert_eq!(
estimator
.estimate_cost(&cc_payload_0, &StacksEpochId::Epoch20,)
.expect("Should be able to provide cost estimate now"),
ExecutionCost {
write_length: 1,
write_count: 1,
read_length: 1,
read_count: 1,
runtime: 1,
}
);

// cc_payload_1 should be updated
assert_eq!(
estimator
.estimate_cost(&cc_payload_1, &StacksEpochId::Epoch20,)
.expect("Should be able to provide cost estimate now"),
ExecutionCost {
write_length: 5,
write_count: 5,
read_length: 5,
read_count: 5,
runtime: 5,
}
);
}

#[test]
/// This tests the PessimisticEstimator as a unit (i.e., separate
/// from the trait auto-impl method) by providing payload inputs
Expand Down

0 comments on commit 98cdf3d

Please sign in to comment.