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

Add contract global state in VM #208

Closed
wants to merge 1 commit into from
Closed

Conversation

dr-orlovsky
Copy link
Member

@dr-orlovsky dr-orlovsky commented Feb 18, 2024

This is WIP on adding contract state (not just operation state, but whole contract state) to the VM required for #201

It seems like this will require refactoring of the validation workflow, since now we must evaluate the contract state not in stdlib, but right during the validation procedure itself...

I think it will be nice to postpone this until v0.12

NB: this is WIP since the global state for the contract IS NOT COMPUTED, i.e. validator always sees an empty contract state.

@dr-orlovsky dr-orlovsky added refactoring Code refactoring *consensus* Issues affecting distributed concensus labels Feb 18, 2024
@dr-orlovsky dr-orlovsky added this to the v0.12.0 milestone Feb 18, 2024
@dr-orlovsky dr-orlovsky requested a review from crisdut February 18, 2024 21:48
Copy link

codecov bot commented Feb 18, 2024

Codecov Report

Attention: Patch coverage is 0% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 15.4%. Comparing base (52e9dcb) to head (7472d06).
Report is 38 commits behind head on master.

Files Patch % Lines
src/validation/validator.rs 0.0% 9 Missing ⚠️
src/validation/logic.rs 0.0% 5 Missing ⚠️
src/vm/op_contract.rs 0.0% 3 Missing ⚠️
src/vm/runtime.rs 0.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #208     +/-   ##
========================================
- Coverage    16.8%   15.4%   -1.3%     
========================================
  Files          32      33      +1     
  Lines        3896    4175    +279     
========================================
- Hits          653     644      -9     
- Misses       3243    3531    +288     
Flag Coverage Δ
rust 15.4% <0.0%> (-1.3%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

crisdut
crisdut previously approved these changes Feb 26, 2024
Copy link
Member

@crisdut crisdut left a comment

Choose a reason for hiding this comment

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

It seems good.

I think it's a good approach to do validation in the next version.

When you merge this PR, I will update my PR and finish implementing the opcodes, ok?

@dr-orlovsky
Copy link
Member Author

@crisdut well, the whole idea is to merge this PR after v0.11 release (i.e. in v0.12), while we need the refactored opcodes in v0.11, thus your PR can't be based on this one

@cymqqqq
Copy link
Contributor

cymqqqq commented Jul 11, 2024

@crisdut well, the whole idea is to merge this PR after v0.11 release (i.e. in v0.12), while we need the refactored opcodes in v0.11, thus your PR can't be based on this one

Do you mean refactor the common opcodes in the aluvm or the contract-level opcodes in the rgb-core vm?

@crisdut
Copy link
Member

crisdut commented Jul 11, 2024

@crisdut well, the whole idea is to merge this PR after v0.11 release (i.e. in v0.12), while we need the refactored opcodes in v0.11, thus your PR can't be based on this one

Do you mean refactor the common opcodes in the aluvm or the contract-level opcodes in the rgb-core vm?

He was referring to the average contract-level opcodes in the RGB, because this PR is about state verifications on the "current state" of the contract.

@cymqqqq
Copy link
Contributor

cymqqqq commented Jul 12, 2024

Yes, I agree with you, we need to refactor the contract-level opcodes, and then maybe there is a lack of the other state types.

@dr-orlovsky
Copy link
Member Author

Closing in favor of #264

@dr-orlovsky dr-orlovsky closed this Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
*consensus* Issues affecting distributed concensus refactoring Code refactoring
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants