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

Simulate transaction should return execution result #5837

Closed
4 tasks
fedekunze opened this issue Mar 19, 2020 · 1 comment · Fixed by #5839
Closed
4 tasks

Simulate transaction should return execution result #5837

fedekunze opened this issue Mar 19, 2020 · 1 comment · Fixed by #5839
Assignees

Comments

@fedekunze
Copy link
Collaborator

Summary

Return result and gasConsumed (or GasInfo) when using ctx.QueryWithData("/app/simulate", txBytes) instead of only gasConsumed.

Problem Definition

#5421 refactored error handling and query logic around tx simulation. In particular handleQueryApp in the "simulate" was changed to return the gas consumed from the simulation instead of the sdk.Result.

Thi change broke one of the Web3 RPC calls, eth_Call, which executes a call without performing a state transition (i.e just like SDK simulation).

Ethermint needs the sdk.Result from this execution in order to unmarshal the result Data.

Proposal

Update handle query app to return sdk.Result and GasInfo by introducing a new type SimulationResponse:

func handleQueryApp(app *BaseApp, path []string, req abci.RequestQuery) abci.ResponseQuery {
	if len(path) >= 2 {
		switch path[1] {
		case "simulate":
			txBytes := req.Data

			tx, err := app.txDecoder(txBytes)
			if err != nil {
				return sdkerrors.QueryResult(sdkerrors.Wrap(err, "failed to decode tx"))
			}

			gInfo, res, _ := app.Simulate(txBytes, tx)

			simRes := sdk.SimulationReponse{
				GasInfo: gInfo,
				Result: res,	
			}

			return abci.ResponseQuery{
				Codespace: sdkerrors.RootCodespace,
				Height:    req.Height,
				Value:     codec.Cdc.MustMarshalBinaryLengthPrefixed(simRes),
			}
...

cc: @austinabell @alexanderbez

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@fedekunze fedekunze changed the title Simulate transaction should return exec result Simulate transaction should return execution result Mar 19, 2020
@fedekunze fedekunze self-assigned this Mar 19, 2020
@alexanderbez
Copy link
Contributor

Be sure to update the logic in the tx builder that decodes this response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants