js_operation_serializer outdates

xeroc commented 5 years ago

As a UI developer I want to have https://github.com/bitshares/bitsharesjs/blob/master/lib/serializer/src/operations.js automatically generated by js_operations_serializer from core.

Impacts Describe which portion(s) of BitShares Core may be impacted by this bug. Please tick at least one box.

[x] API (the application programming interface)
[x] Build (the build process or something prior to compiled code)
[ ] CLI (the command line wallet)
[ ] Deployment (the deployment process after building such as Docker, Travis, etc.)
[ ] DEX (the Decentralized EXchange, market engine, etc.)
[ ] P2P (the peer-to-peer network for transaction/block propagation)
[ ] Performance (system or user efficiency, etc.)
[ ] Protocol (the blockchain logic, consensus, validation, etc.)
[ ] Security (the security of system or user data, etc.)
[x] UX (the User Experience)
[x] Other (please add below) JS client development

Cross references:

https://github.com/bitshares/bitshares-core/issues/947

CORE TEAM TASK LIST

[ ] Evaluate / Prioritize Bug Report
[ ] Refine User Stories / Requirements
[ ] Define Test Cases
[ ] Design / Develop Solution
[ ] Perform QA/Testing
[ ] Update Documentation

xeroc commented 5 years ago

Still missing: #947

abitmore commented 5 years ago

There was a (IMHO incomplete) PR https://github.com/bitshares/bitshares-core/pull/1133. @xeroc perhaps your new code will make it obsolete.

pmconrad commented 5 years ago

It was my understanding that the sole purpose of js_operation_serializer is to describe API data structures in a way that can be parsed by bitshares-js.

If the output of js_operation_serializer is different from what bitshares-js uses today, that opens up two questions:

How is the operations.js file in bitshares-js maintained today?
Is js_operation_serializer still required at all? Or put differently: would it be better/easier/desirable for bitshares-js maintenance if the output of js_operation_serializer matched operations.js?

clockworkgr commented 5 years ago

I believe there is some manual work involved
Probably easier if they did

pmconrad commented 5 years ago

@sschiessl-bcp can you comment? Do you need the serializer? Which output format do you need / would you prefer?

sschiessl-bcp commented 5 years ago

The serializer comes it handy for adjusting and should be maintained. I have not checked how it deviates, but I assume it is only in the formatting since we have introduced the prettifier hook.

Best case would be if it matches of course, but it would also be accepable to run the prettifier hook manually once before comparing it.

xeroc commented 5 years ago

This pull request was mostly generated by the serializer in this PR: https://github.com/bitshares/bitsharesjs/pull/43/files

pmconrad commented 5 years ago

Should the serializer produce output identical to that file then?

xeroc commented 5 years ago

Should the serializer produce output identical to that file then?

That would probably be the best, I only made changes so that the output is no longer in CoffeeScript but rather in ES/Javascript. Someone else would need to follow up on this.

You can find a quick diff here: http://www.mergely.com/ITVZr6ep/

Ignore the stuff at the top. Down the file, lots of whitespace/wrapping differences that could potentially be resovled by jslint

abitmore commented 5 years ago

I only made changes so that the output is no longer in CoffeeScript but rather in ES/Javascript.

There is a document mentioned a step to convert generated coffee script to javascript: https://github.com/bitshares/bitshares-core/issues/1701#issuecomment-482651340

pmconrad commented 5 years ago

Edited OP to make this a user story.

pmconrad commented 5 years ago

See #1133 for some steps in the right (I hope) direction.

abitmore commented 5 years ago

Mentioned in bitsharesjs: https://github.com/bitshares/bitsharesjs/blob/master/lib/serializer/src/operations.js#L59-L61

// ##  Generated code follows
// # programs/js_operation_serializer > npm i -g decaffeinate
// ## -------------------------------

xeroc commented 5 years ago

@abitmore thanks for the comment. Closing this pull request.

pmconrad commented 5 years ago

As commented in #1702 I'd like to keep this open for a complete solution.

dot5enko commented 5 years ago

i've been involved into adding htlc features to ui so this also affected adding this ops to bitsharesjs. From what i've seen - everything works perfect, except extensions field of ops. as of now they get converted by default as future_extension type in js, and in most cases - it didn't work. Extensions in c++ have their own type - struct *_extension_type which could be converted into js i guess instead of generic extension "constructor" which you now need to write by your own. I can take over this issue in core project as i wanted to connect to core development process and this issue corefer to my interests now in bitshares-ui.

PS. now output of json_operation_serializer is not usable in bitsharesjs even in any of its PR implementation

pmconrad commented 5 years ago

There are currently no extensions defined for HTLC operations. The extensions field is just a placeholder for future extensions to be added. Insofar the representation as future_extension sounds correct, but I'm not familiar with the js library types.

In any case it would be great if someone from the UI team could take up this issue. You guys know best what you need.

dot5enko commented 5 years ago

@pmconrad im talking not about htlc concretly here. serializer doesn't handle extensions at all. Now output of this program cannot be used directly in bitsharesjs

bitshares / bitshares-core

js_operation_serializer outdates #1701

Cross references:

CORE TEAM TASK LIST