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

Fix wrong type for comment key #2250

Merged
merged 6 commits into from
Dec 17, 2019
Merged

Fix wrong type for comment key #2250

merged 6 commits into from
Dec 17, 2019

Conversation

i1skn
Copy link
Contributor

@i1skn i1skn commented Dec 13, 2019

Description

Fix wrong type for comment key

Tested

Android

Other changes

Fix the same issue in verification flow

Related issues

Backwards compatibility

Yes

Copy link
Contributor

@annakaz annakaz left a comment

Choose a reason for hiding this comment

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

Great job catching this 💪 left some minor comments

packages/mobile/src/identity/commentKey.ts Show resolved Hide resolved
// currentDEK is actually a string instead of an array
eqAddress(currentWalletAddress, address) &&
currentDEK &&
eqAddress([currentDEK].join(), dataKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, but is it possible to fix the type when getDataEncryptionKey is called in line 522 without losing parallelization? Would be nice to be consistent with the other change (and to make it easy to search getDataEncryptionKey once the type signature in contractkit is fixed)

@codecov
Copy link

codecov bot commented Dec 16, 2019

Codecov Report

Merging #2250 into master will increase coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2250      +/-   ##
==========================================
+ Coverage   74.89%   74.89%   +<.01%     
==========================================
  Files         274      274              
  Lines        7802     7803       +1     
  Branches      985      701     -284     
==========================================
+ Hits         5843     5844       +1     
  Misses       1846     1846              
  Partials      113      113
Flag Coverage Δ
#mobile 74.89% <50%> (ø) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/identity/commentKey.ts 38.46% <0%> (ø) ⬆️
packages/mobile/src/identity/verification.ts 80.79% <100%> (+0.1%) ⬆️

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 9ab55eb...232c78f. Read the comment docs.

@i1skn
Copy link
Contributor Author

i1skn commented Dec 16, 2019

@annakaz PTAL - I've addressed the feedback.

@i1skn i1skn merged commit d905c82 into master Dec 17, 2019
@i1skn i1skn deleted the i1skn/wrong-type-comment-key branch December 17, 2019 09:53
aaronmgdr added a commit that referenced this pull request Dec 17, 2019
* master:
  Adding slashing multiplier to validators.sol and reward calculation (#2239)
  Update genesis value, show name in election:list, fix assertRevert (#2216)
  Audit fixes for Reserve (#1816)
  Adapting the script to the last documentation changes (#2241)
  Bump excon from 0.65.0 to 0.71.0 in /packages/mobile (#2274)
  Fix wrong type for comment key (#2250)
  Fix/block gas limit exceeded #2 (#2263)
  Stop running broken end-to-end mobile test (#2096)
  [Wallet] Fix non integer wei issue in exchange (#2277)
  Fix validatorgroup:list bug by not fetching unneeded info (#2257)
  [Wallet] New exchange trade screen and fix ts build in celotool (#2167)
  Update Role on About Page (#2271)

# Conflicts:
#	packages/web/src/about/team/team-list.ts
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.

User is blocked to proceed for Transaction and Exchange.
3 participants