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

Eth API: On revert, return data in the error #10311

Closed
5 of 15 tasks
Stebalien opened this issue Feb 17, 2023 · 5 comments · Fixed by #12553
Closed
5 of 15 tasks

Eth API: On revert, return data in the error #10311

Stebalien opened this issue Feb 17, 2023 · 5 comments · Fixed by #12553
Assignees
Labels

Comments

@Stebalien
Copy link
Member

Stebalien commented Feb 17, 2023

Checklist

  • This is not a new feature or an enhancement to the Filecoin protocol. If it is, please open an FIP issue.
  • This is not a new feature request. If it is, please file a feature request instead.
  • This is not brainstorming ideas. If you have an idea you'd like to discuss, please open a new discussion on the lotus forum and select the category as Ideas.
  • I have a specific, actionable, and well motivated improvement to propose.

Lotus component

  • lotus daemon - chain sync
  • lotus miner - mining and block production
  • lotus miner/worker - sealing
  • lotus miner - proving(WindowPoSt)
  • lotus miner/market - storage deal
  • lotus miner/market - retrieval deal
  • lotus miner/market - data transfer
  • lotus client
  • lotus JSON-RPC API
  • lotus message management (mpool)
  • Other

Improvement Suggestion

JSON-RPC supports an optional data field in errors to return additional information. Furthermore, it looks like Ethereum clients use this field to return "revert reasons".

Specifically, we have an error type defined here:

https://github.com/filecoin-project/go-jsonrpc/blob/787a96afc5da60084c97b597b61f0fad18c65a3d/handler.go#L68-L72

type respError struct {
	Code    ErrorCode       `json:"code"`
	Message string          `json:"message"`
	Meta    json.RawMessage `json:"meta,omitempty"`
}

We should extend this to:

type respError struct {
	Code    ErrorCode       `json:"code"`
	Message string          `json:"message"`
	Meta    json.RawMessage `json:"meta,omitempty"`
	Data    json.RawMessage `json:"data,omitempty"`
}

And provide a way to return said data from a JSON-RPC method. Likely the best way is to define interface { ErrorData() interface{} }.

@Stebalien
Copy link
Member Author

@jennijuju jennijuju added this to the v1.21.1 milestone Feb 22, 2023
@jennijuju jennijuju modified the milestones: v1.21.1, v1.21.0 Feb 22, 2023
@jennijuju jennijuju removed this from the v1.21.0 milestone Mar 16, 2023
@ZenGround0 ZenGround0 removed the FEVM label Jun 12, 2023
@aarshkshah1992
Copy link
Contributor

@virajbhartiya This will be a fun one and has been a long pending ask from users. Want to pick this up ?
Will need changes to Lotus and https://github.com/filecoin-project/go-jsonrpc.

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Sep 30, 2024

@Stebalien Which ETH APIs should be returning reverted reason in the data field ? Is it the eth_call and eth_estimateGas APIs only ?

@aarshkshah1992
Copy link
Contributor

@virajbhartiya is picking this up.

@Stebalien
Copy link
Member Author

@Stebalien Which ETH APIs should be returning reverted reason in the data field ? Is the eth_call and eth_estimateGas APIs only ?

As far as I know, yes.

@BigLep BigLep added this to FilOz Sep 30, 2024
@github-project-automation github-project-automation bot moved this to 📌 Triage in FilOz Sep 30, 2024
@rjan90 rjan90 moved this from 📌 Triage to 🐱 Todo in FilOz Oct 3, 2024
@rjan90 rjan90 moved this from 🐱 Todo to ⌨️ In Progress in FilOz Oct 3, 2024
@github-project-automation github-project-automation bot moved this from ⌨️ In Progress to 🎉 Done in FilOz Oct 25, 2024
@rjan90 rjan90 moved this from 🎉 Done to ☑️ Done (Archive) in FilOz Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging a pull request may close this issue.

5 participants