This repository has been archived by the owner on Nov 6, 2020. It is now read-only.
Give accounts precedence over address_book entries #3732
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jacogr
added
A0-pleasereview 🤓
derhuerst
reviewed
Dec 7, 2016
| @@ -37,7 +37,7 @@ class IdentityName extends Component { | |||
| render () { | |||
| const { address, accountsInfo, tokens, empty, name, shorten, unknown, className } = this.props; | |||
| const account = accountsInfo[address] || tokens[address]; | |||
| const hasAccount = account && (!account.meta || !account.meta.deleted); | |||
| const hasAccount = account && (account.uuid || !account.meta || !account.meta.deleted); | |||
There was a problem hiding this comment.
I think isAccount would be more intuitive here.
There was a problem hiding this comment.
hasAccount is perfectly acceptable (as it is in the current codebase) since it indicates that the account is in the acocuntsInfo list.
Wontfix.
There was a problem hiding this comment.
In addition https://github.com/ethcore/parity/pull/3739 removes the line (in this specific file as well as all the others that has the same hasAccount check & naming completely)
rphmeier
reviewed
Dec 7, 2016
| @@ -57,7 +57,7 @@ impl<C: 'static> ParityAccounts for ParityAccountsClient<C> where C: MiningBlock | |||
| let info = try!(store.accounts_info().map_err(|e| errors::account("Could not fetch account info.", e))); | |||
| let other = store.addresses_info().expect("addresses_info always returns Ok; qed"); | |||
There was a problem hiding this comment.
unrelated but this proof seems a bit strange
|
Rust side LGTM |
gavofyork
added
A5-grumble 🔥
|
marked grumble from @derhuerst comment |
|
Will be removed anyway. |
derhuerst
added
A8-looksgood 🦄
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
parity_accountsInfoto return entries where accounts have a higher/overriding priority over address_book.json entries