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

Spv implementation #17

Open
wants to merge 113 commits into
base: tx-creation
Choose a base branch
from

Conversation

OrfeasLitos
Copy link
Member

No description provided.

@@ -14,10 +14,46 @@ var TrustDB = require("./trust_db");
var DirectTrust = require("./direct_trust");

class TrustIsRisk {
node : bcoin$FullNode
node : (bcoin$FullNode | bcoin$SPVNode)
Copy link
Member Author

Choose a reason for hiding this comment

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

node : bcoin$Node

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -14,10 +14,46 @@ var TrustDB = require("./trust_db");
var DirectTrust = require("./direct_trust");

class TrustIsRisk {
node : bcoin$FullNode
node : (bcoin$FullNode | bcoin$SPVNode)
Copy link
Member

Choose a reason for hiding this comment

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

Check if you can type this with the superclass

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

// secp256k1 curve: y^2 = x^3 + 7
0x05, 0x5d, 0x5f, 0x28, 0x5e, 0xd7, 0x9d, 0x0c,
0x6f, 0x61, 0xc3, 0x0e, 0xfc, 0x9d, 0x21, 0x91,
0x65, 0x82, 0x80, 0x59, 0xa6, 0x01, 0x25, 0x0c, // 32 bytes with the y coordinate
Copy link
Member

Choose a reason for hiding this comment

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

Keep lines short, move comments above the lines they're describing

Copy link
Member Author

@OrfeasLitos OrfeasLitos Sep 27, 2017

Choose a reason for hiding this comment

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

Done

0x54, 0x72, 0x75, 0x73, 0x74, 0x20, 0x69, 0x73,
0x20, 0x52, 0x69, 0x73, 0x6b, 0x00, 0x00, 0x00, // only x is given in short version
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01];
Copy link
Member

Choose a reason for hiding this comment

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

DRY, compress using above constants

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed compressed version entirely

this.fakePubKeyArray = [0x04, // constant 0x04 prefix
0x54, 0x72, 0x75, 0x73, 0x74, 0x20, 0x69, 0x73,
0x20, 0x52, 0x69, 0x73, 0x6b, 0x00, 0x00, 0x00, // 32 bytes with the x coordinate
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // containing ASCII "Trust is Risk"
Copy link
Member

Choose a reason for hiding this comment

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

Deduce this from the string 'Trust is Risk' instead of typing it out fully; at least the x part

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed compressed versions completely

this.tag = Buffer.from(this.fakeKeyRing.getAddress("base58"));


this.compressedFakePubKeyArray = [0x02, // 0x02 prefix for even y values
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove compressed vars

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

compressedTag : Buffer

constructor(node : (bcoin$FullNode | bcoin$SPVNode)) {
this.fakePubKeyArray = [0x04, // constant 0x04 prefix
Copy link
Member Author

Choose a reason for hiding this comment

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

Indent comment correctly

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

compressedTag : Buffer

constructor(node : (bcoin$FullNode | bcoin$SPVNode)) {
this.fakePubKeyArray = [0x04, // constant 0x04 prefix
Copy link
Member Author

@OrfeasLitos OrfeasLitos Sep 26, 2017

Choose a reason for hiding this comment

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

Put the constants out of the class. Use corresponding --harmony flags in the package.json file

Copy link
Member Author

Choose a reason for hiding this comment

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

@dionyziz There's a million flags, I went one time through them all but they seem to do something else entirely. I read a bit about ESnext and I understood that's a completely separate transpiler (?) that we're currently not using. Can you give me a pointer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Managed to do it without new flags. Check if the result is OK.

test/helpers.js Outdated
@@ -82,9 +81,10 @@ var testHelpers = {
});
},

getOneOfTwoMultisigOutput: (originPubKey, destPubKey, value) => {
getOneOfThreeMultisigOutput: (originPubKey, destPubKey, value) => {
tag = (new TrustIsRisk.TrustIsRisk(new bcoin.fullnode({}))).fakePubKey;
Copy link
Member Author

Choose a reason for hiding this comment

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

use the const that is outside the TrustIsRisk class

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

test/helpers.js Outdated
@@ -82,9 +81,10 @@ var testHelpers = {
});
},

getOneOfTwoMultisigOutput: (originPubKey, destPubKey, value) => {
getOneOfThreeMultisigOutput: (originPubKey, destPubKey, value) => {
tag = (new TrustIsRisk.TrustIsRisk(new bcoin.fullnode({}))).fakePubKey;
Copy link
Member Author

Choose a reason for hiding this comment

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

tag -> fakePubKey

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

test/helpers.js Outdated
return new bcoin.primitives.Output({
script: bcoin.script.fromMultisig(1, 2, [originPubKey, destPubKey]),
script: bcoin.script.fromMultisig(1, 3, [originPubKey, destPubKey, tag]),
Copy link
Member Author

Choose a reason for hiding this comment

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

tag -> fakePubKey

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@@ -0,0 +1,27 @@
# Thanks to http://code.activestate.com/recipes/577821-integer-square-root-function/ for isqrt()
Copy link
Member Author

Choose a reason for hiding this comment

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

move to new directory etc

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

});

it("is a valid bitcoin address", () => {
assert(bcoin.primitives.Address.fromBase58(tir.tag.toString("ascii")));
Copy link
Member Author

@OrfeasLitos OrfeasLitos Sep 26, 2017

Choose a reason for hiding this comment

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

bcoin.primitives.Address.fromBase58(tir.tag.toString("ascii")).should.be.ok

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

var sinon = require("sinon");
var should = require("should");
var fixtures = require("./fixtures");
var assert = require("assert");
Copy link
Member Author

Choose a reason for hiding this comment

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

remove

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@OrfeasLitos OrfeasLitos closed this Oct 1, 2017
@OrfeasLitos OrfeasLitos reopened this Oct 1, 2017
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.

2 participants