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

Rename EcdsaRecoverEcdsaRecovery #1064

Merged
merged 1 commit into from
Dec 2, 2021
Merged

Conversation

cmichi
Copy link
Collaborator

@cmichi cmichi commented Dec 2, 2021

@ascjones I'm also thinking about if we should rename ink_env::ecdsa_recover(…) to something like ink_env::ecdsa_recovery(…) or ink_env::recover_ecdsa(…). What does the native speaker say?

@codecov-commenter
Copy link

codecov-commenter commented Dec 2, 2021

Codecov Report

Merging #1064 (3af5441) into master (44c51d1) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1064   +/-   ##
=======================================
  Coverage   78.75%   78.75%           
=======================================
  Files         248      248           
  Lines        9371     9371           
=======================================
  Hits         7380     7380           
  Misses       1991     1991           
Impacted Files Coverage Δ
crates/engine/src/ext.rs 72.60% <0.00%> (ø)
...tes/env/src/engine/experimental_off_chain/impls.rs 54.12% <0.00%> (ø)
crates/env/src/engine/off_chain/impls.rs 42.37% <0.00%> (ø)
crates/lang/src/env_access.rs 12.00% <ø> (ø)

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 44c51d1...3af5441. Read the comment docs.

@paritytech-ci
Copy link

🦑 📈 ink! Example Contracts ‒ Changes Report 📉 🦑

These are the results when building the examples/* contracts from this branch with cargo-contract 0.16.0-78ea802 and comparing them to ink! master:

Δ Optimized Size Δ Used Gas Total Optimized Size Total Used Gas
accumulator 1.74 K
adder 2.87 K
contract-terminate 1.50 K 200_571
contract-transfer 8.02 K 727
delegator 9.95 K 6_151
dns 21.69 K 18_864
erc1155 47.30 K 54_061
erc20 10.34 K 6_414
erc721 36.21 K 66_677
flipper 2.02 K 173
incrementer 1.94 K 166
multisig 46.25 K 63_165
proxy 3.90 K 2_112
rand-extension 5.14 K 5_553
subber 2.89 K
trait-erc20 27.50 K 15_834
trait-flipper 1.83 K 168
trait-incrementer 1.93 K 332

Link to the run | Last update: Thu Dec 2 07:56:27 CET 2021

@ascjones
Copy link
Collaborator

ascjones commented Dec 2, 2021

@ascjones I'm also thinking about if we should rename ink_env::ecdsa_recover(…) to something like ink_env::ecdsa_recovery(…) or ink_env::recover_ecdsa(…). What does the native speaker say?

On the basis that the fn name should be a verb recover and that ecdsa is the name of the algorithm rather than referring to the public key which is being recovered, I think the current ink_env::ecdsa_recover(…) makes the most sense.

@cmichi cmichi merged commit 582083a into master Dec 2, 2021
@cmichi cmichi deleted the cmichi-rename-ecsda-recover branch December 2, 2021 10:03
xgreenx pushed a commit to Supercolony-net/ink that referenced this pull request Feb 8, 2022
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.

4 participants