Retrieval Deals, Spec V0

[hannahhoward]

Goals

Implement spec v0 retrieval (unixFS only)

Implementation

• Rewrite write retrieval execution as a state machine
  • Test state handlers
• handle payments when needed
• handle absorb and verify blocks as needed
• utilize block consumer / verifiers from old pre-spec

For Discussion

Note the state transitions return a state modifier -- the intent is ultimately to have that be a state transition function in a real FSM type module
Also there's a likely hood states themselves will get written to disk for resuming requests.

[shannonwells]

I have the one question about double subscriber notification and the rest are strictly a judgment call. I didn't mark it request changes in case I missed something.

[shannonwells]

Is xerrors something we should standardize on? I've been using github.com/pkg/errors. I noticed cbor_gen uses it.

[hannahhoward]

We should use xerrors ideally, and wrap errors whenever additional information might prove useful

[shannonwells]

Is this right? It looks like if status is non-terminal, subscribers will be notified both of the status and then ClientEventError. Should there be a return above?

[hannahhoward]

So the status at this line must be terminal -- cause the break in the above loop requires that.

And that notification for status inside the loop is below the break so that the "in progress" notification only goes out if the status is not terminal after applying dealModifier.

my notion of how things will proceed in the loop based on the code above is:

• at start of loop, status is non terminal
• apply logic based on starting status
• apply modifier for new status
• check if new status is terminal, if so exit loop
• notify in progress status update and start loop again, but only if status is non-terminal

and the code at the bottom is:
with terminal status that exited loop--

• if success notify complete
• if terminal error, notify error

Does that track with your understanding of this code?

[shannonwells]

Ok that makes sense.

[shannonwells]

These numbers don't have meaning just reading through the test - I had to search around to figure out where they came from. It would be helpful to compare with the test variables up above to compare, including totals, e.g.

require.Equal(t, dealState.BytesPaidFor, defaultTotalReceived)
require.Equal(t, dealState.CurrentInterval, defaultCurrentInterval+defaultIntervalIncrease)

[shannonwells]

😸

[hannahhoward]

💸