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

OpenCAP sending feature added #409

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

OpenCAP sending feature added #409

wants to merge 15 commits into from

Conversation

wagslane
Copy link

#374

Added the option for users to send directly to OpenCAP aliases. An OpenCAP alias is human-readable, looks like an email address, and can represent BTC/BCH addresses even as they update for each transaction. OpenCAP is an open-source protocol, and as such anyone can run their own OpenCAP server (it would be awesome if bitcoin.com hosted one at some point in the future)

For testing purposes, feel free to use https://ogdolo.com as it is a fully DNSSEC secured free OpenCAP server.

OpenCAP protocol: https://github.com/opencap/protocol

@jbdtky
Copy link
Contributor

jbdtky commented Oct 30, 2018

Thanks for your proposal. Our team will let you know.

@@ -0,0 +1,160 @@
'use strict';
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Your service doesn't follow our standards.
    https://github.com/Bitcoin-com/Wallet/blob/master/src/js/services/shapeshift.service.js
    The link below is an example of our standard for a service.
  2. If you want to use a promise, use the promise from Angular1 provided by $q.
    Example
    var deferred = $q.defer(); ... deferred.resolve(res); deferred.reject(err); ... return deferred.promise;
  3. If you want make a http request, please use $http from Angular1 instead of fetch.
  4. file naming convention opencap.service.js
  5. declare your module in bitcoincom.services

Copy link
Author

Choose a reason for hiding this comment

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

Perfect. I'll work on this, thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Just finished the update. I covered all of your points, let me know if there is anything else you want changed!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I will review it and let you know. I don't guarantee anything, I will review technically if it is suitable to our app.

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good. I'll keep an eye if you need more changes made.

@wagslane
Copy link
Author

wagslane commented Dec 3, 2018

Just finished adding the CashAddr format, as well as a more friendly UI/UX for aliases (they now use the same input field)

@jchau207
Copy link
Contributor

Can one of the admins verify this patch?


var parseAddresses = function(respData, dnssec) {
let addresses = {}
return $q((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : => : lambda expression is not supported for old javascript engine.
Should be :

function onQ() {
}

parseSRV(response.data)
.then(data => getAddresses(alias, data.host, data.dnssec))
.catch(function(error) {
return $q((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : => : lambda expression is not supported for old javascript engine.
Should be :

function onQ() {
}

function getAddress(alias) {
let aliasData = validateAlias(alias);
if (aliasData.username === '' || aliasData.domain === '') {
return $q((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : => : lambda expression is not supported for old javascript engine.
Should be :

function onQ() {
}

.then(function(response) {
deferred.resolve(
parseSRV(response.data)
.then(data => getAddresses(alias, data.host, data.dnssec))
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : => : lambda expression is not supported for old javascript engine.
Should be :

function onThen() {
}

popover.show(angular.element(document.querySelector('#search-input')))
});
})
.catch(status => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : => : lambda expression is not supported for old javascript engine.
Should be :

function onCatch() {
}

return deferred.promise;
}

var parseSRV = function(respData) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : var parseSRV = function(
Should be : function parseSRV(

});
};

var getAddresses = function(alias, host, dnssec) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : var getAddresses = function(
Should be : function getAddresses(

let deferred = $q.defer();
$http
.get(`https://${host}/v1/addresses?alias=${alias}`)
.then(function(response) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : function(
Should be : function onThen(

.then(function(response) {
deferred.resolve(parseAddresses(response.data, dnssec).then());
})
.catch(function(response) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : function(
Should be : function onCatch(

return deferred.promise;
};

var parseAddresses = function(respData, dnssec) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Now : var parseAddresses = function(
Should be : function parseAddresses(

Copy link
Author

Choose a reason for hiding this comment

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

I think I got it all addressed

@jbdtky
Copy link
Contributor

jbdtky commented Jan 7, 2019

We have written this var parsed = bitcoinUriService.parse(text);
The perfect place for calling your protocol should be in the bitcoinUriService, you should not need to change anything else (src/js/controllers/tab-send.controller.js)

@wagslane
Copy link
Author

wagslane commented Jan 7, 2019

We have written this var parsed = bitcoinUriService.parse(text);
The perfect place for calling your protocol should be in the bitcoinUriService, you should not need to change anything else (src/js/controllers/tab-send.controller.js)

I actually don't think it belongs there... Looks like that functions returns an object, where I need to return a promise right?

@wagslane
Copy link
Author

Just fixed a merge conflict that came up recently. Have you had a chance to look at my question above? I think they way I have it may be correct (needs to be a promise returned)

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.

5 participants