Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add reserved exit codes and remove commented-out ones #2100

Closed
wants to merge 1 commit into from

Conversation

elmattic
Copy link
Contributor

@elmattic elmattic commented Jan 10, 2025

This PR exposes reserved exit codes and removes others that are likely unnecessary.

The rationale is that for certain endpoints in the Ethereum RPC-API (e.g., Filecoin.EthTraceBlock), aligning Forest's JSON output with Lotus's output is important.

Having these exit codes available will help in the forest-tool api compare tool.

See go-state-types exit codes and traceErrMsg that we want to match in Forest.

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.96%. Comparing base (7f8986c) to head (5cf7672).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2100   +/-   ##
=======================================
  Coverage   75.96%   75.96%           
=======================================
  Files         154      154           
  Lines       15771    15771           
=======================================
  Hits        11980    11980           
  Misses       3791     3791           
Files with missing lines Coverage Δ
shared/src/error/mod.rs 40.54% <ø> (ø)

@elmattic elmattic requested review from anorth and Stebalien and removed request for anorth January 10, 2025 15:06
@Stebalien
Copy link
Member

Really, these shouldn't have been exported anywhere. Are you actually seeing these error codes?

@Stebalien
Copy link
Member

If you are running into them in practice, we need to rename those to something else like (deprecated X).

@elmattic
Copy link
Contributor Author

Are you actually seeing these error codes?

I believe I've seen some of them while testing Filecoin.EthTraceBlock with Lotus on Calibnet.

I'm good if we don't defined them. Let's close this PR then.

@Stebalien
Copy link
Member

SGTM. If you do notice one, please report a bug and we'll rename it to something sane (i.e., "deprecated X" instead of "reserved X").

@Stebalien Stebalien closed this Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants