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

feat: Charge for Create metadata #618

Merged
merged 51 commits into from
Oct 31, 2023

Conversation

bvrooman
Copy link
Contributor

@bvrooman bvrooman commented Oct 28, 2023

Related Issues:

This PR extends the gas costs to include costs for contract state root and contract id.

The gas costs are not final and will need to be calculated by running benchmarks on a dedicated instance.

@bvrooman bvrooman force-pushed the bvrooman/feat/charge-for-create-metadata branch from 1780190 to 8c1ab35 Compare October 30, 2023 16:35
@bvrooman bvrooman marked this pull request as ready for review October 30, 2023 18:16
@bvrooman bvrooman self-assigned this Oct 30, 2023
@bvrooman bvrooman requested a review from a team October 30, 2023 18:23
Base automatically changed from bvrooman/feat/charge-for-input-signature-recovery to master October 30, 2023 18:32
@bvrooman bvrooman changed the title feat: Charge for Create metadata feat: Charge for Create metadata Oct 30, 2023
let state_root_length = storage_slots.len() as Word;
let state_root_gas = gas_costs.state_root.resolve(state_root_length);

let contract_id_gas = gas_costs.contract_id;
Copy link
Collaborator

Choose a reason for hiding this comment

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

contract_id calculation is simply the calling of the hash functions. You can use dependent cost for hash function instead

@xgreenx xgreenx added this pull request to the merge queue Oct 31, 2023
Merged via the queue into master with commit 35a8f29 Oct 31, 2023
37 checks passed
@xgreenx xgreenx deleted the bvrooman/feat/charge-for-create-metadata branch October 31, 2023 17:11
@xgreenx xgreenx mentioned this pull request Oct 31, 2023

let contract: Option<Contract> = witnesses
.get(*bytecode_witness_index as usize)
.map(|c| c.as_ref().into());
Copy link
Member

Choose a reason for hiding this comment

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

Should we just get the len here? Based on the type annotation above is seems like we're creating an owned copy of the contract which is going to add a lot more overhead to the gas calculation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I can update that in follow up PR.

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

Successfully merging this pull request may close these issues.

3 participants