feat(node): support read only txn #1291
Conversation
|
@cryptoAtwill closes #1236 ? |
| msg, | ||
| ApplyKind::Implicit, | ||
| raw_length, | ||
| true, |
There was a problem hiding this comment.
NIT: Maybe we can have:
const WITH_REVERT: bool = true;
and the use it here instead? Just to not have a magical boolean here.
There was a problem hiding this comment.
For the case here, I think a bool might just work because there are just two cases.
There was a problem hiding this comment.
It doesn't matter the number of cases, when in calling code foo(true, false, true) bears no meaning while foo(ExecPersistence::Revert, ExecPersistence::Apply, ExecPersistence::Revert) does. Ignore the specific naming.
There was a problem hiding this comment.
understand where you are coming from. Updated the naming slightly to reflect the changes you mentioned. If I use an enum here, I still have to convert to bool. Will take note of this for future changes.
There was a problem hiding this comment.
Reviewable status: 0 of 14 files reviewed, 6 unresolved discussions (waiting on @cryptoAtwill, @karlem, and @LePremierHomme)
ipc/provider/src/config/deserialize.rs line 44 at r1 (raw file):
{ struct Visitor; impl serde::de::Visitor<'_> for Visitor {
These are clippy artifacts from the rust version bump. We should strive to separate them out
| msg, | ||
| ApplyKind::Implicit, | ||
| raw_length, | ||
| true, |
There was a problem hiding this comment.
Reviewable status: 0 of 14 files reviewed, 6 unresolved discussions (waiting on @drahnr, @karlem, and @LePremierHomme)
ipc/provider/src/config/deserialize.rs line 44 at r1 (raw file):
Previously, drahnr (Bernhard Schuster) wrote…
These are clippy artifacts from the rust version bump. We should strive to separate them out
understand, I will create a separate PR next time. This one I think it's still manageable, so I just put them into one.
| msg, | ||
| ApplyKind::Implicit, | ||
| raw_length, | ||
| true, |
There was a problem hiding this comment.
For the case here, I think a bool might just work because there are just two cases.
There was a problem hiding this comment.
Reviewable status: 0 of 14 files reviewed, 6 unresolved discussions (waiting on @cryptoAtwill, @karlem, and @LePremierHomme)
Cargo.toml line 146 at r2 (raw file):
reqwest = { version = "0.11.13", features = ["json"] } sha2 = "0.10" serde = { version = "1.0.217", features = ["derive"] }
Any particular reason we need this patch version
| msg, | ||
| ApplyKind::Implicit, | ||
| raw_length, | ||
| true, |
There was a problem hiding this comment.
It doesn't matter the number of cases, when in calling code foo(true, false, true) bears no meaning while foo(ExecPersistence::Revert, ExecPersistence::Apply, ExecPersistence::Revert) does. Ignore the specific naming.
There was a problem hiding this comment.
Reviewable status: 0 of 14 files reviewed, 6 unresolved discussions (waiting on @drahnr, @karlem, and @LePremierHomme)
Cargo.toml line 146 at r2 (raw file):
Previously, drahnr (Bernhard Schuster) wrote…
Any particular reason we need this patch version
It's having crate conflict with ref-fvm, ref-fvm requires a higher version.
| msg, | ||
| ApplyKind::Implicit, | ||
| raw_length, | ||
| true, |
There was a problem hiding this comment.
understand where you are coming from. Updated the naming slightly to reflect the changes you mentioned. If I use an enum here, I still have to convert to bool. Will take note of this for future changes.
This PR adds the functionality of read only txn that can perform queries over the latest in memory changes of the fvm execution state.
The read only execution is delegated to
fvm. Also it bumps the rust version to 1.83.0 as there were some crates conflict.Fixed a few clippy issues.
This change is