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

Update Integer Square Root Function #117

Merged
merged 13 commits into from
Aug 10, 2022
Merged

Conversation

chgorman
Copy link
Contributor

Scope

Update Integer Square Root function and add additional tests for gas consumption.

Why?

This version of integer square root function has proof of convergence;
it also reduces gas from an average of 1283 to 1259 per call.

@chgorman chgorman requested a review from nelsonhp July 31, 2022 21:22
@github-actions github-actions bot added javascript Pull requests that update Javascript code solidity labels Jul 31, 2022
@codecov
Copy link

codecov bot commented Jul 31, 2022

Codecov Report

Merging #117 (dbb216c) into main (464a465) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #117      +/-   ##
==========================================
+ Coverage   53.23%   53.25%   +0.01%     
==========================================
  Files         337      337              
  Lines       42984    42991       +7     
  Branches      360      362       +2     
==========================================
+ Hits        22883    22895      +12     
+ Misses      17671    17670       -1     
+ Partials     2430     2426       -4     
Impacted Files Coverage Δ
bridge/contracts/libraries/math/Sigmoid.sol 100.00% <100.00%> (ø)
badgerTrie/smt_merkle_proof.go 69.53% <0.00%> (+3.90%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@vtleonardo
Copy link
Contributor

vtleonardo commented Aug 1, 2022

This is awesome @chgorman ! Overall the PR LGTM, just fix the formatting issues on the new contract. You can do it by running:

cd bridge
npm run format

If you don't like the output of the linter on this file, just put it to be ignored in the ./bridge/.solhintignore

@chgorman
Copy link
Contributor Author

chgorman commented Aug 3, 2022

I ran the linter. I do think the previous version is better as it is a bit easier to understand in some parts (like the bit length computation), but that is fine.

@chgorman chgorman requested a review from a team as a code owner August 8, 2022 15:44
@ghost ghost added the t/bug Something isn't working label Aug 8, 2022
@nelsonhp nelsonhp merged commit 43e3006 into alicenet:main Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code solidity t/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants