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: thread-safe vm #3752

Merged

Conversation

zhangsoledad
Copy link
Member

@zhangsoledad zhangsoledad commented Dec 12, 2022

What problem does this PR solve?

Upgrade ckb-vm to v0.23.2

What is changed and how it works?

nervosnetwork/ckb-vm#299
According to nervosnetwork/ckb-vm#299, ckb-vm adds Send and Sync bound to InstructionCycleFunc, Debugger, Syscalls, and goes further to thread-safe by eliminating lifetime.

The CKB side upgrade, need to rewrite the Syscalls to be Send or Sync.
There are some gray areas where some types are theoretically not Sync, such as store_transaction, and for this PR it would be too far to go to fix these issues, currently we can guarantee that store_transaction will not be modified concurrently, so it is safe.

Related changes

  • PR to update owner/repo:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Release note

Title Only: Include only the PR title in the release note.

@zhangsoledad zhangsoledad force-pushed the zhangsoledad/upgrade-vm branch from 3a2a584 to fafe86a Compare December 19, 2022 08:21
@zhangsoledad zhangsoledad marked this pull request as ready for review December 19, 2022 09:28
@zhangsoledad zhangsoledad requested a review from a team as a code owner December 19, 2022 09:28
@zhangsoledad zhangsoledad requested review from quake and removed request for a team December 19, 2022 09:28
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/upgrade-vm branch from 584b18a to 859f98f Compare January 16, 2023 01:23
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/upgrade-vm branch from 859f98f to 7d03975 Compare March 1, 2023 08:25
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/upgrade-vm branch from 7d03975 to 3aa3051 Compare March 2, 2023 02:19
@zhangsoledad zhangsoledad added this pull request to the merge queue Mar 2, 2023
Merged via the queue into nervosnetwork:develop with commit f9cf6db Mar 2, 2023
@zhangsoledad zhangsoledad deleted the zhangsoledad/upgrade-vm branch March 2, 2023 06:22
@doitian doitian mentioned this pull request Apr 13, 2023
3 tasks
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