forest

forest copied to clipboard

Issue summary

In most of the places of the protocol, we are using the "current AMT" which allows setting custom bit width. This was not the case some time ago. Now, the serialized form includes this bit width: https://github.com/filecoin-project/ref-fvm/blob/1dc5bb17e494d5be7ad5b1aa4aa27576484fee0a/ipld/amt/src/root.rs#L32-L42

It was a breaking change, I believe first introduced here https://github.com/filecoin-project/go-amt-ipld/pull/38. Then the Rust implementation https://github.com/ChainSafe/forest/pull/978 was adapted as well.

The issue is that we still need to be able to deserialize the old messages and some mechanisms in the nodes still rely on the old format, e.g. in Lotus: https://github.com/filecoin-project/lotus/blob/master/chain/sync.go#L420

We can't fully get rid of the old Forest implementation and just use ref-fvm one for the moment we temporarily revived the old format here. It's a clear workaround though. If there would be another Rust node implementation, it would need to have a similar nasty hack.

What we need is some kind of support for the old format using the ref-fvm codebase without copying it back into our own.

One rough attempt can be found here: https://github.com/filecoin-project/ref-fvm/compare/master...legacy-amt-support In its current state, it's not even close to being pushed as a PR (defaults don't fully work, naming, a bunch of internals exposed etc.) but it's an idea. By default, we should use the new shiny serializer, but in certain cases highlighted above the old one with the default bitwidth. Happy to see any alternatives though.

This task will most likely require changes in the ref-fvm code.

Task summary

• [ ] Discuss the issue with the ref-fvm team
• [ ] If change is in ref-fvm, implement it and wait/push for a release (perhaps only fvm_ipld_amt can be done fast)
• [ ] Implement the change in Forest
• [ ] Remove legacy AMT implementation from Forest codebase
• [ ] Check if Forest is able to sync

Acceptance Criteria

• [ ] change is globally accepted in both Forest and FVM team
• [ ] Legacy AMT is removed from Forest
• [ ] our node is still syncing

Other information and links

We have a green light to implement this feature on fvm_ipld_amt side. The support for AMTv0 would live on the current version (no major bump) and any new breaking changes (like adding a bitwidth field) would result in a new major version.

For anyone picking this issue (if it's not me), feel free to continue my linked attempt or contrive something else. It won't be a major version change so it has to be backwards compatible.

Relevant PR: https://github.com/filecoin-project/ref-fvm/pull/925

@creativcoder A new release of fvm_ipld_amt has been published. Could you check if it solves our problem and report back here with your findings?

Closed via #2580