Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

abi: import files from latest geth release #815

Merged
merged 68 commits into from
Jan 8, 2021

Conversation

yoomee1313
Copy link
Contributor

@yoomee1313 yoomee1313 commented Dec 10, 2020

Proposed changes

This PR updates abi package and abigen. It applies all abi related changes up to v1.9.21.

abi, bind package is identical with geth v1.9.21. However, backends package is not exactly identical because some geth changes are not applied yet. I didn’t applied them or applied part of them because those changes are huge and not only included at abi package. Also, I removed unnecessary codes such as vyper, clef, etc. You can refer the commit msgs which geth PR is applied.

Below PRs are the PR lists that i didn’t included. If needed, please refer them.

  • internal/ethapi: return revert reason (geth #21083)
  • all: seperate consensus error and evm internal error (geth #20830)
  • eth/filters, interfaces.go: EIP-234 Add blockHash option to eth_getLogs (geth #16734)
  • accounts/abi/bind/backend, internal/ethapi: recap gas limit with balance (geth #21043, geth #21346)

fix NewFallback TC
NewFallback TC contains a bug. Fixed TC and fixed NewFallback contract to below.
Related Geth PR ethereum/go-ethereum#22053

pragma solidity ^0.6.2;

contract NewFallbacks {
  event Fallback(bytes data);
  fallback() external { 
      emit Fallback(msg.data);
  }
  event Received(address addr, uint value); 
  receive() external payable {
      emit Received(msg.sender, msg.value);
  }
}

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...

yumiel added 30 commits December 4, 2020 16:31
Allow the --abi flag to be given - to indicate that it should read the
ABI information from standard input. It expects to read the solc output
with the --combined-json flag providing bin, abi, userdoc, devdoc, and
metadata, and works very similarly to the internal invocation of solc,
except it allows external invocation of solc.

This facilitates integration with more complex solc invocations, such
as invocations that require path remapping or --allow-paths tweaks.

Simple usage example:

    solc --combined-json bin,abi,userdoc,devdoc,metadata *.sol | abigen --abi -
plus fix typo: binruntimesFlag -> binruntimeFlag, RCode -> RuntimeCode
also, not imported unuseful folders: cmd/geth, cmd/swarm
They are translated as [24]byte
* Add Java template version
* accounts/abi/bind: fix merge issue
* Fix CI
argument type and name were reversed
accounts/abi, cmd/abigen: support tuple
* accounts/abi/bind, cmd/abigen: add objc back
* accounts/abi/bind: use byte[24] as function indicator, resolve struct slice or array, remove sort logic
accounts: fix issues in abi
accounts/abi: address comment
* accounts, abigen: link dependent libs in deploy
* abigen: add java generation
* bind: Fix unit tests
* abigen: add unit test
* Fix CI
* cmd, common: refactor abigen command line interface
* cmd/abigen: address comment
from "all: on-chain oracle checkpoint syncing (#19543)"
The abi package already supports function overload by adding a suffix to the overloaded function name, but it uses the function name with suffix to calculate signature(both for the event and method).

This PR fixes it by adding a new field named RawName, which can be used to calcuate all signatures but use Name to distinguish different overloaded function.
* accounts/abi: fix various issues
The fixed issues include:
(1) If there is no return in a call function, unpack should
return nil error
(2) For some functions which have struct array as parameter,
it will also be detected and generate the struct definition
(3) For event, if it has non-indexed parameter, the parameter
name will also be assigned if empty. Also the internal struct
will be detected and generate struct defition if not exist.
(4) Fix annotation generation in event function
* accounts/abi: add new abi field internalType
* accounts: address comments and add tests
* accounts/abi: replace strings.ReplaceAll with strings.Replace
* accounts/abi/bind: use store function to remove code duplication
* accounts/abi/bind: removed unused type defs
* accounts/abi/bind: error on tuples in topics
* Cosmetic changes to restart travis build
Currently the java generator generates invalid input on pure/view functions
that have no return type. e.g. `function f(uint u) view public {}`
This is not a problem in practice as people rarely ever write functions like this.
yumiel added 6 commits December 14, 2020 10:12
* UnpackRevert function is came from geth PR (seperate consensus error and evm internal error #20830)
* abi.go copyright is updated due to import statement modification
* move topics.go and topics_test.go to abi package
* add TestWaitDeployedCornerCases test case
also modify copyright of doc.go
* fix typos
package backends refactoring
* bring NewSimulatedBackendWithDatabase and refactor NewSimulatedBackendWithGasPrice and NewSimulatedBackend function
* bring nullSubscription and refactor filterBackend related functions
* change variable name from statedb to stateDB
* remove unused parameters
Copy link
Contributor

@KimKyungup KimKyungup left a comment

Choose a reason for hiding this comment

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

I compared this abi pkg code with geth reference code with @yoomee1313.
There is some difference. But @yoomee1313 will list up the PRs which is not applied in this PR. They caused some differences.

And I reviewed extra code also. I think it's ok.

However, the new fallback test code doesn't work.
@yoomee1313 Please check the test.
I think there may be some differences in the transaction call routine.

yumiel added 2 commits December 16, 2020 00:32
* bring gasCap code to simulatedBackend EstimateGas (from geth PR #21043)
* take into account gas price during gas estimation (from geth PR #20189)
* make bind.logger to be public so it can be used at backends package too
* initialize gotEvent after receive call
* remove assembly call in solidity code, instead use msg.data
@yoomee1313 yoomee1313 force-pushed the abi-wholeCodeImporting branch from 3446389 to c77bed7 Compare December 21, 2020 00:53
Refactor NewFallback TC.

Co-authored-by: Ethan (Kyungup Kim) <[email protected]>
KimKyungup
KimKyungup previously approved these changes Dec 21, 2020
@KimKyungup
Copy link
Contributor

KimKyungup commented Dec 21, 2020

@yoomee1313 In content, all geth PR link is broken. Could you link them with their PRs?

ehnuje
ehnuje previously approved these changes Dec 21, 2020
@yoomee1313 yoomee1313 dismissed stale reviews from ehnuje and KimKyungup via 6b6f8f1 January 3, 2021 18:28
@ehnuje
Copy link
Contributor

ehnuje commented Jan 4, 2021

@yoomee1313 Could you please share the progress of this PR? Are there any other things left to merge this change?

@yoomee1313
Copy link
Contributor Author

Forgive me for being late. I was delayed trying to find out about the situations below.
(1) NewFallback fix verification has been completed.
-> In the original code, calldata was copied to memory using calldatacopy and then copied to the event through parameters.
-> However, there was a problem in the above process, so the structure has been changed to copy msg.data to the event through parameters.
(2) The execution of the OpenZeppelin Test Suite has been completed.
-> In the openzeppelin test, it was checked whether this PR had an effect on the revert reason error. The error is that the revert reason is not returned.
-> The conclusion is that the error is not affected by this PR. Instead, it is related to the PRs (especially eth_call revert reason) which are described in the above description. We will complete the support for this in the future with next incompatibility change.

In conclusion, this PR supports solidity 0.6 including New Fallback and struct type.
Please take a look! @KlaytnDev

@KimKyungup
Copy link
Contributor

PTAL @aidan-kwon. Let's merge~!!!

@yoomee1313 yoomee1313 merged commit 9e729c3 into klaytn:dev Jan 8, 2021
@yoomee1313 yoomee1313 deleted the abi-wholeCodeImporting branch January 8, 2021 01:56
@yoomee1313 yoomee1313 restored the abi-wholeCodeImporting branch January 8, 2021 02:07
@yoomee1313 yoomee1313 deleted the abi-wholeCodeImporting branch January 8, 2021 02:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants