feat(dursto): make functions using Resolver return Result by NSant215 · Pull Request #770 · tezos/riscv-pvm

NSant215 · 2026-02-02T13:56:08Z

Part of RV-892.
Part of RV-894.

What

Refactor methods using the resolver to return a Result rather than absolute value (though keeping the Error type to Infallible for now, with generic types being introduced in a later PR.

Why

Moves us closer to being able to use the resolver for blinding and lazy loading nodes.

The next step will be to make the Id type and error types generic. This will be done in #795

How

Manually Testing

make all

Tasks for the Author

Link all Linear issues related to this MR using magic words (e.g. part of, relates to, closes).
Eliminate dead code and other spurious artefacts introduced in your changes.
Document new public functions, methods and types.
Make sure the documentation for updated functions, methods, and types is correct.
Add tests for bugs that have been fixed.
Explain changes to regression test captures when applicable.
Write commit messages in agreement with our guidelines.
Self-review your changes to ensure they are high-quality.
Complete all of the above before assigning this MR to reviewers.

codecov · 2026-02-02T14:06:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.09%. Comparing base (214b4ab) to head (64719e8).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #770      +/-   ##
==========================================
- Coverage   91.10%   91.09%   -0.01%     
==========================================
  Files         110      110              
  Lines       20913    20913              
  Branches    20913    20913              
==========================================
- Hits        19052    19050       -2     
- Misses       1488     1490       +2     
  Partials      373      373

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

NSant215 · 2026-02-11T17:08:35Z

I think it would be a good idea to add a commit with a test that uses a fallible implementation of Resolver and asserts that the differentiation between missing nodes and failed resolutions works. It would also help to enforce future changes to Resolver don't depend on the ArcResolver semantics.

@kurtisc I have included such a test in #795 - it requires the type change that @thomasathorne was asking for on the Resolver trait that I am already doing in that MR. Let me know if this is ok

durable-storage/benches/avl_tree.rs

vapourismo · 2026-02-11T17:50:19Z

durable-storage/src/avl/node.rs

        {
            let right = node_mut.right_mut();
-            let (mut min, _, shrank) = Tree::take_min(right, resolver);
+            let (mut min, _, shrank) = Tree::take_min(right, resolver)?;


I don't think this method is soundly structured any more. The effect of the mutation that occurs on this line is not undone in case of a failure below. This means, when a failure occurs, the tree is in an inconsistent state. I would imagine this could turn into a foot-gun quite easily. Additionally, avoiding potential exploits to happen by making this correct by construction would probably pay off nicely.

NSant215 · 2026-02-12T17:45:21Z

@vapourismo @thomasathorne @kurtisc @emturner @zcabter I have rebased on Ole's error refactor branch that has removed the need for infallible everywhere.

NSant215 · 2026-02-13T09:43:07Z

durable-storage/src/avl/node.rs

+
+    let data = resolved.to_encode(resolver)?;
+    let computed = Hash::hash_encodable(data).expect("The hashing should not fail");
+    let _ = resolved.hash.set(computed); // ignore race: another thread may have set it


I think this is a possible solution - as the hash will be set in either case, but I don't know if another thread setting it will mean that the hash set will not be matching the node then, but not sure of how else to do this

vapourismo · 2026-02-13T15:47:38Z

durable-storage/benches/avl_tree.rs

I'm curious what you intend to signal with the let _ = ... pattern. I've only come across it when a #[must_use] return value needs to be ignored. Though, I am not convinced that's the case here.

sorry have fixed here

vapourismo · 2026-02-13T15:49:28Z

durable-storage/src/avl/node.rs

+    pub(crate) fn to_encode(
+        &self,
+        resolver: &impl Resolver<Arc<Node>, Node>,
+    ) -> Result<impl Encode + '_, OperationalError> {


I wish you had introduced the resolver hash method before this, so you wouldn't need to make this encoder fallible.

I didn't think that making the hash method as part of the resolver would be able to make it infallible?

vapourismo · 2026-02-13T15:55:16Z

durable-storage/src/avl/node.rs

+        node: &mut Arc<Node>,
+        resolver: &mut impl Resolver<Arc<Node>, Node>,
+    ) -> Result<(), OperationalError> {
+        let node_mut = resolver.resolve_mut(node).expect("Node must exist.");


Suggested change

let node_mut = resolver.resolve_mut(node).expect("Node must exist.");

let node_mut = resolver.resolve_mut(node)?;

NSant215 force-pushed the santnr/resolver branch from 918ae3c to 8c424c7 Compare February 2, 2026 16:35

NSant215 force-pushed the santnr/resolver-fallible branch 2 times, most recently from f090b73 to d8748ee Compare February 3, 2026 09:46

NSant215 force-pushed the santnr/resolver branch from 0880df7 to e4f6d71 Compare February 3, 2026 09:46

NSant215 force-pushed the santnr/resolver-fallible branch from d8748ee to 8bd8997 Compare February 3, 2026 14:08

NSant215 force-pushed the santnr/resolver branch from e4f6d71 to edf7c47 Compare February 3, 2026 14:08

NSant215 force-pushed the santnr/resolver-fallible branch from 8bd8997 to 7b08ced Compare February 3, 2026 14:30

NSant215 force-pushed the santnr/resolver branch from edf7c47 to ba6c4a2 Compare February 3, 2026 17:01

NSant215 force-pushed the santnr/resolver-fallible branch from 7b08ced to 046bb04 Compare February 3, 2026 17:01

NSant215 force-pushed the santnr/resolver branch from ba6c4a2 to 37a8a75 Compare February 4, 2026 10:46

NSant215 force-pushed the santnr/resolver-fallible branch 2 times, most recently from edfacfe to ebad67d Compare February 4, 2026 11:05

NSant215 force-pushed the santnr/resolver branch from 37a8a75 to 8cc254f Compare February 4, 2026 11:05

NSant215 force-pushed the santnr/resolver-fallible branch from ebad67d to b9d2fef Compare February 4, 2026 11:39

NSant215 force-pushed the santnr/resolver branch from 8cc254f to 4ccf7c5 Compare February 4, 2026 11:39

NSant215 force-pushed the santnr/resolver-fallible branch from b9d2fef to bd05f39 Compare February 4, 2026 11:43

NSant215 force-pushed the santnr/resolver branch from 4ccf7c5 to b6eb296 Compare February 4, 2026 11:43

NSant215 force-pushed the santnr/resolver-fallible branch from bd05f39 to d792b08 Compare February 4, 2026 13:42

NSant215 force-pushed the santnr/resolver branch from b6eb296 to 94cc4b9 Compare February 4, 2026 13:42

NSant215 force-pushed the santnr/resolver-fallible branch from d792b08 to 3b3c0e7 Compare February 4, 2026 13:50

NSant215 force-pushed the santnr/resolver branch from 94cc4b9 to bc56143 Compare February 4, 2026 13:50

NSant215 force-pushed the santnr/resolver-fallible branch 2 times, most recently from f0b16d3 to 53297e8 Compare February 5, 2026 09:52

NSant215 force-pushed the santnr/resolver branch from bc56143 to 3f400f3 Compare February 5, 2026 09:52

Base automatically changed from santnr/resolver to main February 5, 2026 13:07

NSant215 force-pushed the santnr/resolver-fallible branch 3 times, most recently from 10b51d1 to 4b20377 Compare February 6, 2026 17:00

NSant215 changed the title ~~feat(dursto): make Resolver trait fallible~~ feat(dursto): make functions using Resolver return Result Feb 6, 2026

NSant215 force-pushed the santnr/resolver-fallible branch from 54a3a4d to ce9027b Compare February 11, 2026 15:09

NSant215 requested review from emturner, kurtisc, thomasathorne, vapourismo and zcabter February 11, 2026 17:09

NSant215 force-pushed the santnr/resolver-fallible branch 2 times, most recently from e3ce608 to 1286291 Compare February 11, 2026 17:21

vapourismo reviewed Feb 11, 2026

View reviewed changes

NSant215 force-pushed the santnr/resolver-fallible branch 3 times, most recently from 0db6457 to 6302eda Compare February 12, 2026 15:23

vapourismo mentioned this pull request Feb 12, 2026

refactor(durable-storage): categorise error types into operational and invalid arguments #814

Merged

9 tasks

NSant215 force-pushed the santnr/resolver-fallible branch from 6302eda to 2415214 Compare February 12, 2026 17:27

NSant215 changed the base branch from main to ole/push-vtywzosmovko February 12, 2026 17:28

NSant215 force-pushed the santnr/resolver-fallible branch from 2415214 to b139a3b Compare February 12, 2026 17:43

NSant215 requested a review from vapourismo February 12, 2026 17:45

NSant215 force-pushed the santnr/resolver-fallible branch from b139a3b to 4807f6f Compare February 13, 2026 09:33

NSant215 commented Feb 13, 2026

View reviewed changes

vapourismo force-pushed the ole/push-vtywzosmovko branch from 2edd59c to e14864c Compare February 13, 2026 10:38

NSant215 force-pushed the santnr/resolver-fallible branch from 4807f6f to a178c3b Compare February 13, 2026 11:34

Base automatically changed from ole/push-vtywzosmovko to main February 13, 2026 13:52

NSant215 force-pushed the santnr/resolver-fallible branch from a178c3b to 0e20387 Compare February 13, 2026 13:59

vapourismo approved these changes Feb 13, 2026

View reviewed changes

NSant215 force-pushed the santnr/resolver-fallible branch from 0e20387 to 242ba46 Compare February 15, 2026 14:45

feat(dursto): make methods using resolver return result

64719e8

NSant215 force-pushed the santnr/resolver-fallible branch from 242ba46 to 64719e8 Compare February 15, 2026 14:47

NSant215 requested a review from vapourismo February 15, 2026 14:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dursto): make functions using Resolver return Result#770

feat(dursto): make functions using Resolver return Result#770
NSant215 wants to merge 1 commit intomainfrom
santnr/resolver-fallible

NSant215 commented Feb 2, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 2, 2026 •

edited

Loading

Uh oh!

NSant215 commented Feb 11, 2026

Uh oh!

Uh oh!

vapourismo Feb 11, 2026

Uh oh!

NSant215 commented Feb 12, 2026

Uh oh!

NSant215 Feb 13, 2026

Uh oh!

vapourismo Feb 13, 2026

Uh oh!

NSant215 Feb 15, 2026

Uh oh!

vapourismo Feb 13, 2026

Uh oh!

NSant215 Feb 15, 2026

Uh oh!

vapourismo Feb 13, 2026

Uh oh!

NSant215 Feb 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	let node_mut = resolver.resolve_mut(node).expect("Node must exist.");
	let node_mut = resolver.resolve_mut(node)?;

Conversation

NSant215 commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

How

Manually Testing

Tasks for the Author

Uh oh!

codecov bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

NSant215 commented Feb 11, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NSant215 commented Feb 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

NSant215 commented Feb 2, 2026 •

edited

Loading

codecov bot commented Feb 2, 2026 •

edited

Loading