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

SDK-621 Generate boilerplate for JS user library setup #176

Merged
merged 33 commits into from
Nov 22, 2019

Conversation

paulyoung
Copy link
Contributor

I'm blocked on an issue unrelated to SDK-621, with sending the canister IDs generated by dfx to the client. Some info is in the commit message, but more details to follow.

This reverts commit eec6a2d.
Now that dfx generates canister IDs as unsigned 64-bit integers (little endian)
we need to make sure we can handle that in JavaScript.

At present, the HTTP handler of the client rejects submit calls with a HTTP
status code of 422, and the message "Could not parse body: invalid type: byte
array, expected u64" despite attempting to send the canister_id field as a
uint64 little endian typed array per the CBOR spec.
@paulyoung
Copy link
Contributor Author

After pulling the latest from master, dfx now generates canister IDs. These are u64, so whatever code I had previously couldn’t handle them because JS only supports 53 bit integers.

I’ve made updates to represent little endian u64 canister IDs as a typed array with a tag of 71 (per the CBOR spec).

When I make a call to the /submit endpoint I get a 422 response code saying:

Could not parse body: invalid type: byte array, expected u64

This is an example request: http://cbor.me/?bytes=D9.D9F7(A7(63(617267)-46(4449444C0000)-65(6E6F6E6365)-4D(01050703070709090102030109)-6A(73656E6465725F736967)-58.40(E339DBA47B7121883DA17A97D5C10686951585CDF8637E8C4A1CFCE59951223E0A63756CB98200B24BFB77D4B5C57D3DE75C6EB5CF07B41D8E6A34BC1ADD7C03)-6B(63616E69737465725F6964)-D8.47(52(307836633937616436613834393032316437))-6B(6D6574686F645F6E616D65)-65(6772656574)-6C(726571756573745F74797065)-64(63616C6C)-6D(73656E6465725F7075626B6579)-58.20(B88F8D6800493ED305CF761987AEDD5943E4BC3196F8CEFE9B847EC925BC2EDD)))

Here it is as hex: d9d9f7a763617267464449444c0000656e6f6e63654d010507030707090905030503046a73656e6465725f73696758409762e3d23ce7afc740237fec43574ba7263d4dc5b7567d4896ac3b74d980ed1fcbee16581cd98adb901f511f3da74cf79c816bfc7b69c0baa52d1fc2a8d1070d6b63616e69737465725f6964d847523078366339376164366138343930323164376b6d6574686f645f6e616d656567726565746c726571756573745f747970656463616c6c6d73656e6465725f7075626b6579582027f1d504be1c63a5916b542f4a0c04a0a6e6e9114b7d1c452ba4532d8cc482b9

Since canister.js has no impact on invoking executables.
},
"dfx": "0.4.0",
"version": 1
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aside from removing deployment_id, these changes are due to using dfx config.

I created SDK-626 to track that.

try {
const reply = await actor.greet();
const reply = await hello.greet();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file now matches what @stanleygjones posted in #153 (comment)

return new borc.Tagged(
CborTag.Uint64LittleEndian,
Buffer.from(`0x${hex}`),
) as CanisterId;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the HTTP handler of the client should recognize this as a u64.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently serde_cbor doesn't support tags: https://docs.rs/serde_cbor/0.10.2/serde_cbor/#limitations

Tags are ignored during deserialization and can't be emitted during serialization. This is because Serde has no concept of tagged values. See: #3

I'm not sure how to proceed. @nmattia, do you have any advice?

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 I fully grasp the issue, sorry! do you have some context?

@hansl
Copy link
Contributor

hansl commented Nov 15, 2019

Major types from the spec is: https://tools.ietf.org/html/rfc7049#section-2.1

Tags are still in draft, so ignore (serde ignores too).

A valid CBOR (sent by the CLI) can be found here. You should try to make a CBOR binary the same as this.

@paulyoung
Copy link
Contributor Author

For reference:

--- a/node_modules/borc/src/encoder.js
+++ b/Users/paulyoung/Downloads/encoder.js
@@ -19,6 +19,7 @@ const UNDEFINED = (constants.MT.SIMPLE_FLOAT << 5) | constants.SIMPLE.UNDEFINED
 const NULL = (constants.MT.SIMPLE_FLOAT << 5) | constants.SIMPLE.NULL

 const MAXINT_BN = new Bignumber('0x20000000000000')
+const MAXINT64_BN = new Bignumber('0xffffffffffffffff')
 const BUF_NAN = Buffer.from('f97e00', 'hex')
 const BUF_INF_NEG = Buffer.from('f9fc00', 'hex')
 const BUF_INF_POS = Buffer.from('f97c00', 'hex')
@@ -117,6 +118,10 @@ class Encoder {
     return this.pushWrite(val, 3, 4)
   }

+  _pushUInt64BE (val) {
+    return this.pushWrite(val.toString(16), 6, 8)
+  }
+
   _pushDoubleBE (val) {
     return this.pushWrite(val, 4, 8)
   }
@@ -267,6 +272,11 @@ class Encoder {
   }

   _pushBigint (obj) {
+    // if less than uint64::MAX, use a uint64 field and return.
+    if (obj.isLessThan(MAXINT64_BN) && obj.isPositive()) {
+      return this._pushUInt8(8, MT.POS_INT) && this._pushUInt64BE(obj);
+    }
+
     let tag = TAG.POS_BIGINT
     if (obj.isNegative()) {
       obj = obj.negated().minus(1)
@@ -480,6 +490,9 @@ class Encoder {
         case 5:
           res.write(result[i], index, length, 'utf8')
           break
+        case 6:
+          res.write(result[i], index, length, 'hex')
+          break
         default:
           throw new Error('unkown method')
       }

@paulyoung
Copy link
Contributor Author

In f2e77a7 I'm pointing to a fork of borc that includes the above changes, but I'm now getting a 400 (Bad Request) because the encoded request seems to be missing some bytes: http://cbor.me/?bytes=d9d9f7a763617267464449444c0000656e6f6e63654d010507030807010803040107056a73656e6465725f7369675840e221baf0c334648f5938ca7c856f4fece25b49ca5ae4a24d92026c682a57fa38ddb255bbabd71a78ef753883334dd9098e3109a6601021d8c7e51a5de7a067086b63616e69737465725f6964087212486c2586edbd6b6d6574686f645f6e616d656567726565746c726571756573745f747970656463616c6c6d73656e6465725f7075626b657958207100b0afe2949c8d2acb994e3f357ac00b53d56b46ce66864b2c4fabf61dc0ce

Tests are also failing now. I temporarily disabled them (locally) in order to build and make a request.

This request body from a test reports "Out of bytes to decode (need at least 20012 bytes more)": http://cbor.me/?bytes=d9d9f7a763617267464449444c0000656e6f6e63654d010507030807010803040107056a73656e6465725f7369675840e221baf0c334648f5938ca7c856f4fece25b49ca5ae4a24d92026c682a57fa38ddb255bbabd71a78ef753883334dd9098e3109a6601021d8c7e51a5de7a067086b63616e69737465725f6964087212486c2586edbd6b6d6574686f645f6e616d656567726565746c726571756573745f747970656463616c6c6d73656e6465725f7075626b657958207100b0afe2949c8d2acb994e3f357ac00b53d56b46ce66864b2c4fabf61dc0ce

This one has "unknown additional information": http://cbor.me/?bytes=d9d9f7a76361726740656e6f6e63654800010203040506076a73656e6465725f73696758406b4c9df5c1fa4e271967e91ba63117d97bc14033726e442de9c6bc11ada36332b9e69c2f652fb437cc48314017af3918aeae6dd3af1c5dfde77f63b8bc8f400b6b63616e69737465725f69640800001c00000001016b6d6574686f645f6e616d656567726565746c726571756573745f747970656463616c6c6d73656e6465725f7075626b657958209b62773323ef41a11834824194e55164d325eb9cdcc10ddda7d10ade4fbd8f6d

Things I suspect are also going to be problematic:

  • Decoder doesn't know what to do with our encoded value (CBOR round trip test fails)
  • Changes to borc use big endian, but we expect to use little endian

@nomeata
Copy link
Contributor

nomeata commented Nov 16, 2019

This looks fishy (from your original post)

      6B                                # text(11)
         63616E69737465725F6964         # "canister_id"
      D8 47                             # tag(71)
         52                             # bytes(18)
            307836633937616436613834393032316437 # "0x6c97ad6a849021d7"

it looks like you have a hex-encoded blob as a blob, instead of just the blob.

@nomeata
Copy link
Contributor

nomeata commented Nov 16, 2019

And I guess the implementation might be behind the spec in the sense that canister ids are still numbers, not blobs, e.g.
https://github.com/dfinity-lab/dfinity/blob/400f232bf0fec643c6cb2c793a118fe7dfe5ed8f/rs/http_handler/src/submit.rs#L44-L50

@paulyoung
Copy link
Contributor Author

Latest issue that I'll pick up tomorrow is Could not parse body: invalid type: map, expected byte array. It looks like records are being encoded as maps.

http://cbor.me/?bytes=D9.D9F7(A7(6C(726571756573745F74797065)-64(63616C6C)-65(6E6F6E6365)-4D(01050704010205060506060001)-6B(63616E69737465725F6964)-1B.81335114F5E7BBCB-6B(6D6574686F645F6E616D65)-65(6772656574)-63(617267)-46(4449444C0000)-6D(73656E6465725F7075626B6579)-B8.20(61(30)-18.E1-61(31)-18.5E-61(32)-16-61(33)-18.BA-61(34)-18.DF-61(35)-18.75-61(36)-0C-61(37)-18.2D-61(38)-18.A2-61(39)-18.E7-62(3130)-18.1E-62(3131)-18.C8-62(3132)-18.A5-62(3133)-18.AD-62(3134)-18.98-62(3135)-18.64-62(3136)-18.30-62(3137)-0D-62(3138)-17-62(3139)-18.C5-62(3230)-18.18-62(3231)-18.E1-62(3232)-18.43-62(3233)-18.FF-62(3234)-18.88-62(3235)-18.53-62(3236)-18.E9-62(3237)-18.FD-62(3238)-18.F0-62(3239)-18.94-62(3330)-18.D1-62(3331)-18.D2)-6A(73656E6465725F736967)-B8.40(61(30)-18.DA-61(31)-05-61(32)-18.F0-61(33)-18.22-61(34)-18.52-61(35)-18.A7-61(36)-18.24-61(37)-06-61(38)-18.64-61(39)-18.DD-62(3130)-18.71-62(3131)-18.55-62(3132)-18.DA-62(3133)-18.BB-62(3134)-18.8E-62(3135)-18.37-62(3136)-18.2A-62(3137)-18.54-62(3138)-18.EB-62(3139)-18.38-62(3230)-18.A8-62(3231)-15-62(3232)-18.74-62(3233)-18.1B-62(3234)-18.26-62(3235)-18.F5-62(3236)-18.D9-62(3237)-18.4F-62(3238)-18.74-62(3239)-18.21-62(3330)-18.99-62(3331)-18.20-62(3332)-18.18-62(3333)-18.45-62(3334)-00-62(3335)-18.DD-62(3336)-18.35-62(3337)-16-62(3338)-18.B6-62(3339)-18.D1-62(3430)-18.29-62(3431)-18.EA-62(3432)-18.F4-62(3433)-18.B4-62(3434)-18.C4-62(3435)-18.B8-62(3436)-18.89-62(3437)-18.72-62(3438)-18.A6-62(3439)-18.4A-62(3530)-18.44-62(3531)-18.7E-62(3532)-18.22-62(3533)-18.9E-62(3534)-18.71-62(3535)-18.E9-62(3536)-15-62(3537)-18.47-62(3538)-18.BB-62(3539)-18.58-62(3630)-18.DF-62(3631)-18.C8-62(3632)-18.70-62(3633)-03)))

@paulyoung
Copy link
Contributor Author

Looks like I need to fix something with sender_pubkey.

@paulyoung paulyoung marked this pull request as ready for review November 19, 2019 19:05
@paulyoung paulyoung requested a review from a team as a code owner November 19, 2019 19:05
@paulyoung
Copy link
Contributor Author

Sorry for all of the noise here. I believe the issues with encoding canister IDs are now resolved.

The purpose of this pull request, as the title says, is to generate some boilerplate on behalf of users so that that may import (a JS interface to) their canisters and interact with them directly.

This is partly in response to #153 (comment)

A before and after view of this can be seen in js-user-library/examples/hello/src/hello/index.js

To achieve this we rely on the same mechanism used to create a new project which uses the template at dfx/assets/language_bindings/canister.js. We could add more templates for additional languages if necessary.

makeHttpAgent,
} from "{js_user_lib}";

const { publicKey, secretKey } = generateKeyPair();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this will need to be updated when we have a better story around this.

// canister IDs. However, simple-cbor does not yet provide deserialization so
// we are using `BigNumber` and `Buffer` types instead of `BigInt` and
// `Uint8Array` (respectively) so that we can use the dignifiedquire/borc CBOR
// decoder.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is noteworthy, and explains many of the changes in this PR.

@@ -38,6 +39,7 @@ drv.overrideAttrs (oldAttrs: {
cp ${motoko.didc}/bin/didc $out
cp ${motoko.rts}/rts/mo-rts.wasm $out
mkdir $out/stdlib && cp -R ${motoko.stdlib}/. $out/stdlib
mkdir $out/js-user-library && cp -R ${dfinity-sdk.packages.js-user-library}/. $out/js-user-library
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This puts the JS user library in ~/.cache/dfinity/... with other things we depend on (for now)

generateKeyPair,
makeActor,
makeHttpAgent,
} from "{js_user_lib}";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment, this ends up being a path to ~/.cache/dfinity/.../js-user-library

@paulyoung
Copy link
Contributor Author

I'm going to merge #153 and then #142, so I'll update the base branch here after that.

@paulyoung paulyoung changed the base branch from paulyoung/js-user-lib-example to master November 20, 2019 19:20
@paulyoung paulyoung merged commit 34ae01a into master Nov 22, 2019
@paulyoung paulyoung deleted the paulyoung/js-user-lib-example-2 branch November 22, 2019 18:28
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.

None yet

4 participants