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

New modexp cost #4278

Merged
merged 3 commits into from
Jul 25, 2017
Merged

New modexp cost #4278

merged 3 commits into from
Jul 25, 2017

Conversation

pirapira
Copy link
Member

This follows the new cost formula ethereum/EIPs@4d4d8fb for modexp precompiled contract.

@codecov-io
Copy link

codecov-io commented Jul 24, 2017

Codecov Report

Merging #4278 into develop will increase coverage by 0.06%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #4278      +/-   ##
===========================================
+ Coverage    67.36%   67.42%   +0.06%     
===========================================
  Files          306      306              
  Lines        23472    23478       +6     
===========================================
+ Hits         15812    15831      +19     
+ Misses        7660     7647      -13
Impacted Files Coverage Δ
libethcore/Precompiled.cpp 96.38% <100%> (+0.28%) ⬆️
libethereum/Block.cpp 85.46% <0%> (-0.25%) ⬇️
test/tools/fuzzTesting/fuzzHelper.cpp 48.82% <0%> (+5.46%) ⬆️

@pirapira pirapira requested review from chfast and gumb0 July 24, 2017 12:58
@gumb0
Copy link
Member

gumb0 commented Jul 24, 2017

No unit tests change was needed - it means they cover only the case for mod & base length <=64. Could we add tests for other two cases?

@@ -160,6 +160,16 @@ namespace
return 8 * (_expLength - 32) + highestBit;
}
}

bigint mult_complexity(bigint const& _x)
Copy link
Member

Choose a reason for hiding this comment

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

I'd name it multComplexity

@pirapira
Copy link
Member Author

Right, the unit tests are only for very small, or very large mod | base.

@pirapira
Copy link
Member Author

I added unit tests for the other two cases.

@pirapira
Copy link
Member Author

AppVeyor build is failing and somehow I cannot restart AppVeyor.

@pirapira
Copy link
Member Author

The tests around modexp is passing in AppVeyor.

@pirapira pirapira merged commit b18e9e4 into develop Jul 25, 2017
@pirapira pirapira deleted the new_modexp_cost branch July 25, 2017 09:20
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.

3 participants