Borsh #75

volovyks · 2022-06-01T20:13:55Z

It is throwing errors for now. Trying to find the reason.

The difference between JSON and borsh serialization is 1 line of code + schema. It will be great to have this logic in NearContract in the future.

gagdiez · 2022-06-03T10:31:11Z

Interesting, it behaves as if it was not finding the exports of borsh... could it be because of how the borsh.js was compiled/created?

ailisp · 2022-06-07T07:22:15Z

@gagdiez is right. the problem is the rollup configuration, it can only import ES modules, we need either package borsh in that format or tweak the rollup config. Enable non ES module imports in rollup, is possible, but that is without tree-shaking, and resulting bytecode would be larger

…ion of borsh

ailisp · 2022-06-07T07:35:53Z

I made an attempt to fix rollup configuration to use borsh, now import serialize/deserialize from borsh works. Now it fails for a different issue: borsh uses Buffer, but that's a nodejs API that quickjs doesn't have. We need either:

make a modifier verison of borsh that doesn't use Buffer
or find a pure JavaScript buffer implementation with only Uint8Array. Quickjs support Uint8Array

gagdiez · 2022-06-07T09:44:33Z

Could we implement a Buffer object as a wrapper of uint8array and import it globally? Or use something like: https://github.com/feross/buffer

Edit: I used "browserify" on nodes_modules/borsh/lib/index.js and managed to make the contract compile. It gave an error with the Schema (StatusMessage missed in schema) so I changed the schema to:

const schema = new Map([[StatusMessage, { kind: 'struct', fields: [['records', { kind: 'map', key: 'string', value: 'string' }]]}]]);

Now I am getting a WebAssembly trap: An 'unreachable' opcode was executed.. Which I guess is the browserify magic failing.

ailisp · 2022-06-08T01:58:37Z

@gagdiez Cool! A good step towards success! Let me further explore on your approach

ailisp · 2022-06-13T08:10:23Z

I'm able to make it compile too, by using https://github.com/feross/buffer, update cli.js to config rollup, use more gas since compiled contracts become bigger.

And mutate the status-message/node_modules/borsh/lib/index.js, add this import:

var Buffer = __importDefault(require('buffer/')).Buffer

The borsh test now fail of this error:

Smart contract panicked: cannot read property \'alloc\' of undefined\\n    at BinaryWriter (build/status-message-borsh.js:6816)\\n    at serialize (build/status-message-borsh.js:7118)\\n    at serialize (build/status-message-borsh.js:7246)\\n    at NewClass (build/status-message-borsh.js:42)\\n    at init (build/status-message-borsh.js:7263)\\n

Looking at build/status-message-borsh.js, it correctly included https://github.com/feross/buffer, but it still doesn't work. When trying to import and log the buffer module directly:

import {Buffer} from 'buffer/index'
...
        near.panic(Buffer))

It prints:

"Smart contract panicked: function Buffer() {\\n    [native code]\\n}"

Which seems work. So I guess, import Buffer in borsh-js should also work, but I'm having trouble to modify and build from source: near/borsh-js#57

ailisp · 2022-06-15T09:26:21Z

test-template is working locally, strange that it doesn't work on CI 🤔

volovyks · 2022-06-15T18:21:02Z

It is failing on my machine as well.
Borsh example tests work (we need to add it to CI after merging master).
Setup should be the same in all the examples. We can achieve this by merging the master to this branch since it has only one test project now. I have tried to do it, but it led to several other problems.

volovyks · 2022-06-15T18:22:34Z

cli/cli.js

-            // commonjs(),
-            babel({ babelHelpers: 'bundled' })
+            babel({ babelHelpers: 'bundled' }),
+            commonjs()


Note: Not sure why are we doing this change. Let's discuss it.

commonjs is used for import non-es packages, if I didn't remember it wrong, import commonjs module doesn't work without it.

volovyks · 2022-06-15T18:25:22Z

examples/status-message/src/status-message-borsh.js

+import { NearContract, NearBindgen, call, view, near } from 'near-sdk-js'
+import { serialize, deserialize } from 'borsh';
+import { panic } from '../../../src/api';
+import { Buffer } from 'buffer/'


Not the best practice. Can we add it as a dependency?

Yes, should do so

volovyks · 2022-06-15T18:27:54Z

examples/template/__tests__/test-template.ava.js

@@ -24,8 +24,8 @@ test.beforeEach(async t => {
    // Deploy and init the counter JS contract
    const counter = await root.createSubAccount('counter');
    let contract_base64 = (await readFile('build/contract.base64')).toString();
-    await counter.call(jsvm, 'deploy_js_contract', Buffer.from(contract_base64, 'base64'), {attachedDeposit: '400000000000000000000000'});
-    await counter.call(jsvm, 'call_js_contract', encodeCall(counter.accountId, 'init', []), {attachedDeposit: '400000000000000000000000'});
+    await counter.call(jsvm, 'deploy_js_contract', Buffer.from(contract_base64, 'base64'), {attachedDeposit: '800000000000000000000000'});


The contract is 37! times bigger now :)
I think it's better not to support Borsh at all. But I believe we will be able to find the solution.

Update: ok, this is because we are packaging all the borsh code to the contract. That is not cool :/

Yes this is how commonjs module worse than esm, rollup cannot handle just importing the used functions. And you found it's even worse that it also packaging all borsh code :(

volovyks · 2022-06-15T18:30:23Z

examples/status-message/package.json

    "test": "ava"
  },
  "author": "Near Inc <hello@nearprotocol.com>",
  "license": "Apache-2.0",
  "dependencies": {
-    "near-sdk-js": "file:../../"
+    "borsh": "https://github.com/near/borsh-js/tarball/import-buffer",


I do not understand how this branch is different and why are we using it here. Can we use fix-build branch instead? (or maybe master if we will merge it).

It has a import {Buffer} from buffer/` in borsh.ts, and borsh.js built from that. Without this line it doesn't work.

volovyks · 2022-06-15T18:36:08Z

package.json

    "@rollup/plugin-babel": "^5.3.1",
-    "@rollup/plugin-commonjs": "^21.0.1",
+    "@rollup/plugin-commonjs": "^22.0.0",


This change helps to fix status-message-collections, but I have no idea why :/

Maybe some bug fixes happened in 22.0?

ailisp · 2022-06-16T02:26:26Z

It is failing on my machine as well.

Perhaps due to commonjs increase template.js size (it depends on lodash). Ah i figure out why it work on my side - i didn't yarn before yarn build, the old node_modules work (why??)

Borsh example tests work (we need to add it to CI after merging master).

Good to hear. Yes.

Setup should be the same in all the examples. We can achieve this by merging the master to this branch since it has only one test project now. I have tried to do it, but it led to several other problems.

Got it. Is the problem only related to template and status-message-borsh?

ailisp · 2022-06-17T05:35:32Z

Status summary: we made borsh work by a series of hacks in this PR. But enabling commonjs modules enlarge the contract size significantly. We plan to ship borsh also in esm format to resolve

ailisp · 2023-09-12T01:51:33Z

Closed as completed by #361

volovyks added 2 commits June 1, 2022 23:13

example with overwritten borsh serialization added

ede0769

Merge branch 'master' into borsh

2c8240c

fix rollup configuration to use borsh, but still need a modified vers…

1666773

…ion of borsh

make cli try to load node_modules/buffer

3c4eb63

ailisp added 4 commits June 15, 2022 15:48

get borsh compiles, but cannot get schema right

1c4343b

finally fix borsh

3ad163a

use https github dependency to run on ci

c22afdb

fix template

55c4e38

volovyks commented Jun 15, 2022

View reviewed changes

ailisp linked an issue Sep 23, 2022 that may be closed by this pull request

borsh support in near-sdk-js #242

Closed

ailisp closed this Sep 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Borsh #75

Borsh #75

volovyks commented Jun 1, 2022 •

edited

Loading

gagdiez commented Jun 3, 2022

ailisp commented Jun 7, 2022

ailisp commented Jun 7, 2022 •

edited

Loading

gagdiez commented Jun 7, 2022 •

edited

Loading

ailisp commented Jun 8, 2022

ailisp commented Jun 13, 2022 •

edited

Loading

ailisp commented Jun 15, 2022

volovyks commented Jun 15, 2022

volovyks Jun 15, 2022 •

edited

Loading

ailisp Jun 16, 2022

volovyks Jun 15, 2022

ailisp Jun 16, 2022

volovyks Jun 15, 2022 •

edited

Loading

ailisp Jun 16, 2022

volovyks Jun 15, 2022

ailisp Jun 16, 2022

volovyks Jun 15, 2022

ailisp Jun 16, 2022

ailisp commented Jun 16, 2022 •

edited

Loading

ailisp commented Jun 17, 2022

ailisp commented Sep 12, 2023

Borsh #75

Borsh #75

Conversation

volovyks commented Jun 1, 2022 • edited Loading

gagdiez commented Jun 3, 2022

ailisp commented Jun 7, 2022

ailisp commented Jun 7, 2022 • edited Loading

gagdiez commented Jun 7, 2022 • edited Loading

ailisp commented Jun 8, 2022

ailisp commented Jun 13, 2022 • edited Loading

ailisp commented Jun 15, 2022

volovyks commented Jun 15, 2022

volovyks Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volovyks Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ailisp commented Jun 16, 2022 • edited Loading

ailisp commented Jun 17, 2022

ailisp commented Sep 12, 2023

volovyks commented Jun 1, 2022 •

edited

Loading

ailisp commented Jun 7, 2022 •

edited

Loading

gagdiez commented Jun 7, 2022 •

edited

Loading

ailisp commented Jun 13, 2022 •

edited

Loading

volovyks Jun 15, 2022 •

edited

Loading

volovyks Jun 15, 2022 •

edited

Loading

ailisp commented Jun 16, 2022 •

edited

Loading