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

address add isContract member #2323

Merged
merged 10 commits into from Jul 19, 2019
Merged

address add isContract member #2323

merged 10 commits into from Jul 19, 2019

Conversation

llwslc
Copy link
Contributor

@llwslc llwslc commented Jun 21, 2019

What does this PR do?
add new opcode isContract
Why are these changes required?

This PR has been tested by:

  • Unit Tests
  • Manual Testing

Follow up

Extra details

TIP44

@llwslc llwslc requested a review from ithinker1991 Jun 21, 2019
@codecov-io
Copy link

@codecov-io codecov-io commented Jun 21, 2019

Codecov Report

Merging #2323 into develop will increase coverage by 0.34%.
The diff coverage is 43.33%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2323      +/-   ##
=============================================
+ Coverage      44.01%   44.36%   +0.34%     
- Complexity      5046     5092      +46     
=============================================
  Files            596      596              
  Lines          36706    36723      +17     
  Branches        3973     3975       +2     
=============================================
+ Hits           16156    16292     +136     
+ Misses         19030    18889     -141     
- Partials        1520     1542      +22
Impacted Files Coverage Δ Complexity Δ
...c/main/java/org/tron/common/runtime/vm/OpCode.java 98% <100%> (+0.01%) 17 <0> (ø) ⬇️
src/main/java/org/tron/core/Wallet.java 21.05% <22.22%> (+0.11%) 77 <0> (ø) ⬇️
src/main/java/org/tron/common/runtime/vm/VM.java 65.99% <71.42%> (+0.04%) 117 <0> (+2) ⬆️
...va/org/tron/common/runtime/vm/program/Program.java 51.17% <75%> (+0.18%) 140 <2> (+2) ⬆️
.../java/org/tron/common/runtime/config/VMConfig.java 84.84% <0%> (+3.03%) 19% <0%> (+1%) ⬆️
...va/org/tron/common/overlay/discover/node/Node.java 62.71% <0%> (+5.08%) 15% <0%> (+3%) ⬆️
...ay/discover/node/statistics/MessageStatistics.java 51.03% <0%> (+5.51%) 14% <0%> (+4%) ⬆️
...ron/common/net/udp/message/UdpMessageTypeEnum.java 94.11% <0%> (+5.88%) 5% <0%> (+1%) ⬆️
...erlay/discover/node/statistics/NodeStatistics.java 42.85% <0%> (+5.95%) 7% <0%> (ø) ⬇️
...tron/common/overlay/discover/node/NodeManager.java 52.2% <0%> (+6.28%) 24% <0%> (+1%) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 285df33...9c6322e. Read the comment docs.

@llwslc llwslc force-pushed the feature/isContract branch 2 times, most recently from b0dcd13 to e4b618b Compare Jul 18, 2019
Copy link
Contributor

@ithinker1991 ithinker1991 left a comment

LGTM

@jeancky jeancky merged commit 056b88a into develop Jul 19, 2019
5 checks passed
Sh11thead added a commit that referenced this issue Aug 14, 2019
address add isContract member

(cherry picked from commit 056b88a)

# Conflicts:
#	src/main/java/org/tron/core/Wallet.java
@shydesky shydesky deleted the feature/isContract branch Nov 13, 2019
@sullof
Copy link

@sullof sullof commented Nov 22, 2019

@Sh11thead
To improve TronWeb and stop users from trying to transfer TRX and assets to a smart contract account we need an API that can be called to check if that address is actually a smart contract or not. Do you plan to add a new HTTP API for that?

@Sh11thead
Copy link
Contributor

@Sh11thead Sh11thead commented Nov 23, 2019

@sullof
This API may be a solution .
/wallet/getcontract?value=
If the non-smart contract address, this interface will report an error.

We are envisioning a new API or a unified transfer method that can transfer both the normal address and the smart contract address.

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

7 participants