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.

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

Cost estimations must reset between 2.0 and 2.05 epoch #2908

Closed
kantai opened this issue Nov 4, 2021 · 1 comment · Fixed by #2923
Closed

Cost estimations must reset between 2.0 and 2.05 epoch #2908

kantai opened this issue Nov 4, 2021 · 1 comment · Fixed by #2923
Assignees

Comments

@kantai
Copy link
Member

kantai commented Nov 4, 2021

Cost estimations for contract-call invocations in 2.05 and 2.00 must be reset -- the costs of contract-calls will all change because of the new cost functions, methods of calculating input, etc.

This should be relatively easy to implement -- the cost estimator should just add an epoch identifier to the key used as a cost identifier:

    fn get_estimate_key(tx: &TransactionPayload, field: &CostField, evaluated_epoch: &StacksEpochId) -> String {
        let tx_descriptor = match tx {
            TransactionPayload::TokenTransfer(..) => "stx-transfer".to_string(),
            TransactionPayload::ContractCall(cc) => {
                let epoch_marker = match evaluated_epoch {
                     StacksEpochId::Epoch10 => "",
                     StacksEpochId::Epoch20 => "",
                     StacksEpochId::Epoch2_05 => ":2.05",
                };
                format!("cc{}:{}.{}", epoch_marker, cc.contract_name, cc.function_name)
            }
            TransactionPayload::SmartContract(_sc) => "contract-publish".to_string(),
            TransactionPayload::PoisonMicroblock(_, _) => "poison-ublock".to_string(),
            TransactionPayload::Coinbase(_) => "coinbase".to_string(),
        };

        format!("{}:{}", &tx_descriptor, field)
    }

This will separate the estimates of functions evaluated in 2.05 from those evaluated in 2.00.

@kantai
Copy link
Member Author

kantai commented Nov 11, 2021

For testing this, I believe it's sufficient to unit test. Basically, add a new test or set of tests in the src/cost_estimates/tests/cost_estimators.rs module that:

  1. supply measurements with the evaluated_epoch = 2.0
  2. fetch estimates using 2.05, asserting that no estimate is available
  3. fetch estimates using 2.00, asserting that an estimate is available
  4. supply different measurements with the evaluated_epoch = 2.05
  5. fetch estimates using 2.05, asserting that the estimate is available, and is different than the 2.00 estimate
  6. fetch estimate using 2.00, asserting that its still the same as it was in step 3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants