ChainSafe
refactor: hardcode the filter list, change command line args
Summary of changes
- Hardcode the blocklist, remove the
filter-listfile. - CLI now accepts multiple filters
- CLI is self-documenting
Usage: forest-tool api compare [OPTIONS] [SNAPSHOT_FILES]...
Arguments:
[SNAPSHOT_FILES]...
Snapshot input paths. Supports `.car`, `.car.zst`, and `.forest.car.zst`
Options:
--forest <FOREST>
Forest address
[default: /ip4/127.0.0.1/tcp/2345/http]
....
Filtering:
--include <INCLUDE>
Include methods that may be excluded by the default exclusions.
The current default exclusions are:
- Filecoin.ChainGetParentReceipts
- Filecoin.ChainGetPath
- Filecoin.ChainGetTipSetAfterHeight
- Filecoin.MinerGetBaseInfo
- Filecoin.StateAccountKey
- Filecoin.StateListMessages
- Filecoin.StateSearchMsg
- Filecoin.StateSearchMsgLimited
- Filecoin.StateVMCirculatingSupplyInternal
- Filecoin.StateWaitMsg
These methods are potentially broken, and should not be used until the root cause is resolved.
--exclude <EXCLUDE>
Exclude methods in addition to the default exclusions
--select <SELECT>
Ignore the default exclusions, and run only these methods
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
- [x] I have performed a self-review of my own code,
- [x] I have made corresponding changes to the documentation,
- [x] I have added tests that prove my fix is effective or that my feature works (if possible),
- [x] I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.
Unifies filtering for RPC methods - slightly cleaner than plumbing around a single file to the integration tests that use it.
The rest is all UX and cruft
Unifies filtering for RPC methods - slightly cleaner than plumbing around a single file to the integration tests that use it.
The rest is all UX and cruft
I appreciate the effort put into this implementation - it looks solid and well-tested. That said, from my perspective, it seems significantly more complex than the one with a simple filter list and as such, more difficult to maintain and use. I also find it counter-productive to re-write features that were just implemented; I believe there are items with higher priority in the backlog.
That said, I won't block it; this tool is used exclusively by Forest maintainers so if they prefer it this way, I'm okay with that.
@aatifsyed We will need a separate BLOCK_LIST for offline RPC server (forest-tool api serve), the current test is problematic and passes with errors. It should fail for RPC methods like Filecoin.NetAddrsListen because it's not connected to the p2p network. Hope this is well supported as well.
See https://github.com/ChainSafe/forest/actions/runs/8378304107/job/22942969692?pr=4086#step:7:144
