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

Clean up left over Bytecode code + some related testing fixes #2607

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

andresilva91
Copy link
Contributor

@andresilva91 andresilva91 commented Oct 11, 2024

Motivation

  • Some code was left over from when we removed the non-blobs bytecode code
  • Some places in tests were not using the bytecode blobs properly

Proposal

Fix those and properly mimic what we're doing with user contract/service code with blobs.

Test Plan

CI

Release Plan

  • Nothing to do / These changes follow the usual release cycle.

@andresilva91 andresilva91 changed the title Clean up left over Bytecode code Clean up left over Bytecode code + some related testing fixes Oct 11, 2024
@andresilva91 andresilva91 force-pushed the 10-11-clean_up_left_over_bytecode_code branch from 9160f86 to e80bfec Compare October 11, 2024 04:13
Base automatically changed from 10-10-miscellaneous_fixes_in_amm_readme to main October 11, 2024 04:26
@andresilva91 andresilva91 force-pushed the 10-11-clean_up_left_over_bytecode_code branch 4 times, most recently from 546edf9 to bbb80da Compare October 11, 2024 19:04
@@ -366,6 +366,7 @@ pub struct ChainRuntimeContext<S> {
execution_runtime_config: ExecutionRuntimeConfig,
user_contracts: Arc<DashMap<UserApplicationId, UserContractCode>>,
user_services: Arc<DashMap<UserApplicationId, UserServiceCode>>,
blobs: Arc<DashMap<BlobId, Blob>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

ChainRuntimeContexts can now remain in memory for a long time, and for any number of blocks. For the contracts and services it makes sense to keep them in memory, because an application that has been used on a chain (e.g. non-fungible) it's likely that it will be used again.
But for other—potentially large—blobs (e.g. NFT data!) it's often the case that they get only used once (maybe the NFT is traded on this chain and moves elsewhere). In that case, we shouldn't keep the blob in memory indefinitely.

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.

2 participants