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 UseOpcodeComputationCost option on debug API #1035

Conversation

hqjang-pepper
Copy link
Contributor

@hqjang-pepper hqjang-pepper commented Sep 14, 2021

Proposed changes

  • In the case of existing code, debug api didn't consider computation cost. Therefore, there was a case where the result of the api was different from the actual result.
  • In addition, I added the UseOpcodeComputationCost option to true in the debug api .
  • Through this, computation cost is calculated during vm execution in the debug api, which makes running the api works fine.

Before

> debug.traceBlockByNumber(67918569,{ tracer: 'fastCallTracer'})
Error: insufficient balance for transfer
    at web3.js:3276:20
    at web3.js:6450:15
    at web3.js:5214:36
    at <anonymous>:1:1

After

> debug.traceBlockByNumber(67918569,{ tracer: 'fastCallTracer'})
[ ...(too long, hid)
, {
    result: {
      calls: [{...}],
      from: "0xef03f7dd5f3dd5a3bd33722bbe8331a93f6072d6",
      gas: "0x162bc8",
      gasUsed: "0x97919",
      input: "0x5a3acc50000000000000000000000000cee8faf64bb97a73bb51e115aa89c17ffa8dd16700000000000000000000000000000000000000000000000000000000491039400000000000000000000000009eaefb09fe4aabfbe6b1ca316a3c36afc83a393f000000000000000000000000000000000000000000000000000000003b9aca0000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c6a2ad8cc6e4a7e08fc37cc5954be07d499e7654",
      output: "0x",
      time: "2.920421ms",
      to: "0xc6a2ad8cc6e4a7e08fc37cc5954be07d499e7654",
      type: "CALL",
      value: "0x0"
    },
    txHash: "0x0c82b17785aaa55e2503dd4bed9a0e2eae4ecd4a8b7f278a75239c5e3edb6cf8"
}, {
    result: {
      calls: [{...}],
      from: "0xef03f7dd5f3dd5a3bd33722bbe8331a93f6072d6",
      gas: "0x162bc8",
      gasUsed: "0x97919",
      input: "0x5a3acc50000000000000000000000000cee8faf64bb97a73bb51e115aa89c17ffa8dd16700000000000000000000000000000000000000000000000000000000491039400000000000000000000000009eaefb09fe4aabfbe6b1ca316a3c36afc83a393f000000000000000000000000000000000000000000000000000000003b9aca0000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c6a2ad8cc6e4a7e08fc37cc5954be07d499e7654",
      output: "0x",
      time: "2.932599ms",
      to: "0xc6a2ad8cc6e4a7e08fc37cc5954be07d499e7654",
      type: "CALL",
      value: "0x0"
    },
    txHash: "0x470d1341440f8dc6ea6ce7f1afd752d22279f11bc283e9b608564dd7192f513e"
}]

Before

> debug.traceTransaction("0x470d1341440f8dc6ea6ce7f1afd752d22279f11bc283e9b608564dd7192f513e")
Error: transaction 51edeac5682fa55ba70d00a3e340bb2894070968ce4617f9ff36dddee0b4b4f3 failed: <nil>
    at web3.js:3276:20
    at web3.js:6450:15
    at web3.js:5214:36
    at <anonymous>:1:1

After

> debug.traceTransaction("0x470d1341440f8dc6ea6ce7f1afd752d22279f11bc283e9b608564dd7192f513e")

..(hid)
, {
      depth: 1,
      gas: 832175,
      gasCost: 0,
      memory: ["0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000080", "0000000000000000000000000000000000000000000000000000000000000000", "5a3acc50000000000000000000000000cee8faf64bb97a73bb51e115aa89c17f", "fa8dd16700000000000000000000000000000000000000000000000000000000", "491039400000000000000000000000009eaefb09fe4aabfbe6b1ca316a3c36af", "c83a393f00000000000000000000000000000000000000000000000000000000", "3b9aca0000000000000000000000000000000000000000000000000000000000", "000000a000000000000000000000000000000000000000000000000000000000", "0000000200000000000000000000000000000000000000000000000000000000", "00000000000000000000000000000000c6a2ad8cc6e4a7e08fc37cc5954be07d", "499e765400000000000000000000000000000000000000000000000000000000"],
      op: "RETURN",
      pc: 385,
      stack: ["000000000000000000000000000000000000000000000000000000005a3acc50", "0000000000000000000000002eb02b608d77873ed301e573fe50cfc8045ec832", "0000000000000000000000000000000000000000000000000000000000000080", "0000000000000000000000000000000000000000000000000000000000000001", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000001", "0000000000000000000000000000000000000000000000000000000000000000", "0000000000000000000000000000000000000000000000000000000000000080"],
      storage: {
        2bd0a199149a1f75d087ebc63fe327df6c624265507362ab8c46f89060bc27b8: "00000000000000000000000000000000000000000054d015f29c99b6bf6b0f5a",
        38bcc0532a2f999be26eefda567dcb89ac257acf64f35e503ab7df226016c2bd: "0000000000000000000000000000000000000000000029bb11840da2d8f7af86",
        5d4839737e4841fcfc5c5fa1a861ba46b97057e41e0b2a6df0bd350b993c543c: "0000000000000000000000000000000000000000000217eb5ca13112403ffc76",
        bae2298232552307e0fdea6843c9e28129aa0c370aebc0b1656e0e413cbec3f1: "00000000000000000000000000000000000000000000023308396452912d351f"
      }
  }]
}

works well!

Types of changes

Please put an x in the boxes related to your change.

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Related issues

  • Please leave the issue numbers or links related to this PR here.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

@@ -208,6 +208,7 @@ func (b *CNAPIBackend) GetEVM(ctx context.Context, msg blockchain.Message, state
vmError := func() error { return nil }

context := blockchain.NewEVMContext(msg, header, b.cn.BlockChain(), nil)
vmCfg.UseOpcodeComputationCost = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GetEVM method is only used in the DoCall method, which is only used in api_public_blockchain.go. The DoCall method is used for EstimateGas and EstimateComputationCost. Like issue #1035, this change was added to prevent the estimation value from being changed in advance by not adding this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DoCall can be called by three different APIs: Call, DoEstimateGas, EstimateComputationCost. Among them, DoEstimateGas, EstimateComputationCost already configure UseOpcodeComputationCost. The other API, Call, intentionally disabled UseOpcodeComputationCost to provide unlimited computation for view functions of smart contracts. Therefore, this change was not appropriate for the current API policy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got the point. I deleted that line, so please take another look. Thanks.

@aidan-kwon aidan-kwon merged commit 9e63864 into klaytn:dev Sep 15, 2021
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.

None yet

4 participants