LDC support in predicates #848
Conversation
This reverts commit bba89e4.
| | JNZB | JNEF | JNEB | CFEI | CFSI | CFE | CFS | GTF | LDC | BSIZ | BLDD => { | ||
| true |
There was a problem hiding this comment.
test needs updating (
Line 694 in 39eb13e
fuel-vm/src/storage/predicate.rs
Outdated
| } | ||
| } | ||
|
|
||
| impl StorageRead<ContractsState> for PredicateStorage { | ||
| impl<D> StorageRead<ContractsState> for PredicateStorage<D> |
There was a problem hiding this comment.
question for my own understanding: in what cases can these operations be called?
There was a problem hiding this comment.
We allow only blobs in the predicate. Any other access to the storage should return an error
Added one simple test to check that we can call blobs and they are executed correctly.
xgreenx
requested review from
xgreenx,
Dentosal and
MitchTurner
as code owners
| @@ -108,6 +108,7 @@ where | |||
| }) if owner == &pk => Some(*witness_index as usize), | |||
| _ => None, | |||
| }) | |||
| .sorted() | |||
There was a problem hiding this comment.
personal question: what is the reason behind this ?
There was a problem hiding this comment.
Just notice going over the code that we call dedup, but it not we expect=) So I added dedup to have behavior that we want
There was a problem hiding this comment.
Yeah, dedup() works on "consecutive identical elements". This bit me many times in the past :)
Good catch.
No. In this change we break the idea that predicates are stateless. This PR enables access to the blobs in the predicate context. Since blobs are monotonic it shouldn't invalidate the predicate. If blob doesn't exist, we return a panic. If blob exists, it will not change later. |
| #[derive(Debug, Clone)] | ||
| pub enum PredicateStorageError { | ||
| /// Storage operation is unavailable in predicate context. | ||
| UnsupportedStorageOperation, |
There was a problem hiding this comment.
nit, and definitely not for this PR. But this could be also a StorageError.
|
I think this has been tested already extensively, it would be nice to have a test that checks that loading blobs in a predicate works as expected. That said, it would be nice to have tests for it. Not blocking. |
The rust-sdk has tests with their new custom blob loader to verify this all works. |
## Linked Issues/PRs FuelLabs/fuel-vm#848 FuelLabs/fuel-vm#849 ## Description <!-- List of detailed changes --> ## Checklist - [x] Breaking changes are clearly marked as such in the PR description and changelog - [x] New behavior is reflected in tests - [ ] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [x] I have reviewed the code myself --------- Co-authored-by: AurelienFT <aurelien.foucault@epitech.eu> Co-authored-by: green <xgreenx9999@gmail.com> Co-authored-by: Ahmed Sagdati <37515857+segfault-magnet@users.noreply.github.com>
Closes #456 Requires #611 VM PR FuelLabs/fuel-vm#848 ### Before requesting review - [x] I have reviewed the code myself --------- Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Enables predicates to load immutable blobs of code from storage. This helps reduce bloating blockspace with large predicates.
Core changes:
$PCcan now jump outside of predicate memory rangeldc,bldd, andbsizinstructions are allowed inside of predicates (but only for blobs, no contracts)