Create payload disperser

[litt3]

Why are these changes needed?

• This PR implements the PUT side of the v2 client
• In service of expediency in getting this PayloadDispersal API merged, I've decided to defer writing unit tests until a future PR
  • integration tests will provide better coverage anyway
  • I want to get this up for review ASAP, since @EthenNotEthan is starting with proxy integration, and it would be nice to have basic APIs set
  • I may end up adding some unit tests before this PR merges after all, depending on how long the review process stretches

Checks

• I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
• I've checked the new test coverage and the coverage percentage didn't drop.
• Testing Strategy
  • Unit tests
  • Integration tests
  • This PR is not tested :(

[samlaf]

prob should mention a default. Typically I define a defaultPayloadClientConfig struct with the default values, and assign those in the constructor for any value that wasn't set.

[litt3]

I think it might be worth adding some guidelines for configuration classes to the style guide.

[litt3]

@samlaf Could you clarify what config architecture you're proposing here? Creating a constructor for the struct? Creating a method which checks and sets missing defaults?

[samlaf]

Added TODO for me to add this to style guide: #1137 (comment)

In terms of my personal preference, I like:

• no constructor for config structs (that's very much against the point of having extendability, which requires using go's default zero value initialization)
• having the constructor validate the config and inject default values over zero values (note: this requires care that zero values are always meaningless, which is not always the case!)
• validation + injecting defualt values can be part of a separate function if it's a large config struct that would overwhelm the rest of constructor logic

[litt3]

no constructor for config structs (that's very much against the point of having extendability, which requires using go's default zero value initialization)

Could you explain what you mean by this? What's the problem with having a constructor for ConfigA, and if ConfigA extends ConfigB, call the constructor for ConfigB in the constructor of ConfigA?

[litt3]

I created some config utilities in 959e4564. LMK what you think

[samlaf]

LGTM! Having a default constructor is totally fine, and is actually a good idea! What I meant is that users shouldn't be constructing the config by using a constructor of the type

func NewConfig(fieldA, fieldB, fieldC, fieldD,...) Config {
   return Config{
      fieldA,
      fieldB,
      ...
      }
}

Because this is dumb

[ethenotethan]

is this actually true?

[litt3]

There is a timeout set on the context being passed into pollBlobStatusUntilCertified.

If that timeout triggers, it will abort this GetBlobStatus call if necessary, and abort the polling loop, right?

[ethenotethan]

let's add a TODO specifying that we'll need to add more in-depth response status processing to derive failover errors

[litt3]

I'm not sure I understand what you mean by "derive failover errors".

Do you mean we will need to expand the types of status returned from the top level SendPayload method, beyond what's included in the existing BlobStatus enum?

[ethenotethan]

so we'll need to interpret terminal status status in a meaningful way to map them into a failover error which tells the proxy to return a status 503 response which a rollup batcher can interpret as "go use other DA"

[litt3]

Hmm. Is it not true that any error in dispersal would require resorting to failover? If not, what class of error would you expect to not result in failover, and what would be the strategy to handle it? Retries? (trying to understand likely outcomes, to be able to write a helpful TODO)

[ethenotethan]

we already have a strategy or framework in-place for this that we can extend from. Would recommend just adding a general todo vs re-articulating a methodology that was months of research

[litt3]

Added the TODO

[samlaf]

The changes that I had requested have been made, so approving.

[ethenotethan]

LGTM