Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

EIP198 - Bigint modexp precompiled contract #4028

Merged
merged 3 commits into from
Apr 7, 2017
Merged

EIP198 - Bigint modexp precompiled contract #4028

merged 3 commits into from
Apr 7, 2017

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Apr 3, 2017

#3615
ethereum/EIPs#198

  • Contract code implementation
  • Calculating the cost
  • Optimize code implementation if boost::multiprecision::powm is too slow

@gumb0 gumb0 added this to the Metropolis milestone Apr 3, 2017
@codecov-io
Copy link

codecov-io commented Apr 3, 2017

Codecov Report

Merging #4028 into develop will increase coverage by 0.33%.
The diff coverage is 92.64%.

@@             Coverage Diff             @@
##           develop    #4028      +/-   ##
===========================================
+ Coverage     65.1%   65.43%   +0.33%     
===========================================
  Files          308      309       +1     
  Lines        22552    22670     +118     
===========================================
+ Hits         14683    14835     +152     
+ Misses        7869     7835      -34
Impacted Files Coverage Δ
libethashseal/genesis/metropolisTest.cpp 100% <ø> (ø) ⬆️
libethcore/ChainOperationParams.h 100% <100%> (ø) ⬆️
libethcore/ChainOperationParams.cpp 100% <100%> (ø) ⬆️
test/unittests/libethcore/PrecompiledTest.cpp 100% <100%> (ø)
libethereum/Account.cpp 80.55% <55.55%> (-1.8%) ⬇️
libethcore/Precompiled.h 66.66% <66.66%> (-8.34%) ⬇️
libethcore/Precompiled.cpp 94.11% <96.29%> (+2.45%) ⬆️
libp2p/Host.cpp 72.36% <0%> (-1.06%) ⬇️
libp2p/Common.cpp 52% <0%> (-0.8%) ⬇️
... and 3 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 83b2679...aaf29c7. Read the comment docs.

@gumb0
Copy link
Member Author

gumb0 commented Apr 5, 2017

Looks like the boost::multiprecision::powm is not slower that the manual implemetation, since we don't know the modulo length and can't get rid of intermediate modulo operations (like we do for the EXP opcode implementation)

@gumb0 gumb0 requested review from chriseth and chfast April 5, 2017 10:22
Copy link
Contributor

@gcolvin gcolvin left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@gcolvin
Copy link
Contributor

gcolvin commented Apr 7, 2017

I don't recall the history here, but why didn't we just add a MODEXP opcode?

@gumb0 gumb0 merged commit 43bae2c into develop Apr 7, 2017
@gumb0 gumb0 removed the needs review label Apr 7, 2017
@chfast
Copy link
Member

chfast commented Apr 9, 2017

This is actually a good question, @gcolvin. Should we go to EIPs with it to ask for clarification when a features go as opcodes and when as precompiled contracts...

@chriseth
Copy link
Contributor

@gcolvin for this particular function, the reason to choose a precompiled contract over an opcode was because the arguments can be larger than 32 bits and it would be weird to have such an opcode fetch from memory. Furthermore, I think the idea is that as much as possible should go to precompiled contracts instead of opcodes, and precompiled contracts have to be pure functions.

@pirapira pirapira deleted the eip198 branch April 27, 2017 14:33
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.

5 participants