Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Check vouch status on appId in addition to contentHash #6719

Merged
merged 2 commits into from
Oct 12, 2017

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Oct 12, 2017

Basically allow lookups on both the contentHash and the appId for vouching. (De-dupe results). Just using the contentHash leads to the situation where the count gets lost when the app is actually updated.

parity 2017-10-12 15-24-58

Screenshot shows dapp with verification from both contentHash and appId.

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. B0-patch labels Oct 12, 2017
}

async attachContract () {
const address = await Contracts.get().registry.lookupAddress('vouchfor');
Copy link
Contributor Author

@jacogr jacogr Oct 12, 2017

Choose a reason for hiding this comment

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

Don't lookup contract address per instance and only create a single contract that is used for queries across stores. (next couple of lines after this). Optimisation as discussed in the previous PR.

return contract;
}

async findVouchers (contentHash, id) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made everything async/await to simplify as per comments from previous PR.

@5chdn 5chdn added this to the Patch milestone Oct 12, 2017
@5chdn 5chdn mentioned this pull request Oct 12, 2017
67 tasks
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 12, 2017
@gavofyork gavofyork merged commit edbbd42 into master Oct 12, 2017
@gavofyork gavofyork deleted the jg-vouchfor-dappid branch October 12, 2017 17:28
jacogr added a commit that referenced this pull request Oct 12, 2017
* Check vouch status on appId in addition to contentHash

* Simplify var expansion
arkpar pushed a commit that referenced this pull request Oct 12, 2017
* Check vouch status on appId in addition to contentHash (#6719)

* Check vouch status on appId in addition to contentHash

* Simplify var expansion

* Merge #6725

* Add updated MethodDecoding from master
arkpar pushed a commit that referenced this pull request Oct 12, 2017
* Check vouch status on appId in addition to contentHash (#6719)

* Check vouch status on appId in addition to contentHash

* Simplify var expansion

* Merge #6725

* Add updated MethodDecoding from master
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants