search

ChainSafe / fil-actor-states

State handling crates for the Filecoin Actors
Apache License 2.0
3 stars 2 forks source link

resolve `frc46_token` ambiguity #245

Closed LesnyRumcajs closed 7 months ago

LesnyRumcajs commented 7 months ago 
          Why does it have the _official_ token whilst in the other datacap states we have a copy-pasted one? Can we always use the crate?

_Originally posted by @LesnyRumcajs in https://github.com/ChainSafe/fil-actor-states/pull/244#discussion_r1478366611_

ruseinov commented 7 months ago

I'm not sure why we did have the copy-pasted one in the first place, but the datacap token is modified to cater for Lotus RPC compat. What needs to be done is:

  1. Use the crate instead.
  2. Introduce TokenExt in forest to cater for RPC compat.
ruseinov commented 7 months ago

There's a discussion here: https://chainsafeio.slack.com/archives/C01SZBSC6KZ/p1707717786883299.

I appreciate that this is a private link, but still keeping it here should we need to find the thread fast.

LesnyRumcajs commented 7 months ago

@ruseinov keep in mind that older release used older crate versions. It might make sense to introduce several flavours of this dependency, just like for FVM.

ruseinov commented 7 months ago

@ruseinov keep in mind that older release used older crate versions. It might make sense to introduce several flavours of this dependency, just like for FVM.

I'm unsure why we should bother with that. Why don't we just update latest fil-actor-states as discussed, release it and integrate into latest forest with TokenStateExt. Older release depended on older crate versions, that is fine. Why several flavours?

LesnyRumcajs commented 7 months ago

We should bother with that if there are breaking changes between the crate releases, same reason why we have FVM2, FVM3, and FVM4. It might not be needed now for the token crate, but it's something we should keep in mind.

ruseinov commented 7 months ago

We should bother with that if there are breaking changes between the crate releases, same reason why we have FVM2, FVM3, and FVM4. It might not be needed now for the token crate, but it's something we should keep in mind.

I'm still unsure why. FVM versioning is a really bad example, it's horrible to deal with and I hope we never have to resort to that kind of thing. I'm pretty sure that if it has been done differently - the compatibility layer could have been much smoother.

I'm unclear on why we can't introduce a breaking change in a new release of the crate and then if people want to use the old version - they can. In any case this consideration warrants a valid use-case, before this materialises - I think we're safe to proceed in a way that we did so far.

I would also argue that before resorting to the FVM approach we should carefully weigh all the pros and cons, I'd rather have several codepaths and versioning WITHIN the crate, than having to deal with 3 versions of the same crate and trying fit them into the same product. I know Lotus isn't the best example, as it's using a different approach and a different language, but so far they have managed to maintain actors compatibility without resorting to anything as extreme as that.

LesnyRumcajs commented 7 months ago

I'm not sure what are you even referring to. Let me rephrase. We may need a dependency like frc46_token_9 = 9.0.0, frc46_token_10 = 10.0.0 and so on, if there are breaking changes between those two. For now, there doesn't seem to be such need. If it's still unclear, let's just drop it.

ruseinov commented 7 months ago

I'm not sure what are you even referring to. Let me rephrase. We may need a dependency like frc46_token_9 = 9.0.0, frc46_token_10 = 10.0.0 and so on, if there are breaking changes between those two. For now, there doesn't seem to be such need. If it's still unclear, let's just drop it.

I wasn't sure what you were referring to, I thought we are talking about fil-actor-states. I reckon we can forgo this discussion until it becomes relevant, then let's discuss it again!

I'm really hoping that there won't be any breaking changes to a crate that's supposed to represent a stabilised standard. But as we all know - hope dies last. :)