dedis / cothority

Scalable collective authority
Other
426 stars 106 forks source link

@cothority/byzcoin meaningful instruction print #2401

Closed nkcr closed 3 years ago

nkcr commented 4 years ago

What this PR does

This PR implements a instruction.getArranged() function on @cothority/byzcoin that returns a general-purpose JSON representation of an instruction. This representation is contract aware in the sense that arguments should have a meaningful string representation, based on each different contract.

The JSON has the following structure:

     *  {
     *      status: 0 | 1,   // Non-zero means an error occured
     *      type: "spawn|invoke|delete|UNKNOWN",
     *      contract: "config|darc|...",
     *      args: [
     *          {
     *              "name": "<NAME>",
     *              "value": "<VALUE>",    // Arg value or its summary
     *              "full": "<FULL VALUE>" // Optional. useful if the argument
     *                                     // have a very long representation.
     *          }
     *          ...
     *      ]
     *  }

This functionality is much needed for the Columbus project (blockchain browser). I suggest we include that as an experimental feature until the end of that project (January 2021), where at that point we will have a definitive structure.

Feel free to discuss, I'm open to discussion.

@ineiti's requests:

ineiti commented 3 years ago

Very nice. The following parts need to be improved, however:

If it's not to be included anytime soon, you can Convert to draft.

nkcr commented 3 years ago

Thank you for the review. I addressed most of the comments:

use interfaces for the classes

I can't, as all the methods are static

functionality: ...

I'd like not to implement too much functionality at first, if that's ok I'd like to include this first iteration and add more on top of that when the use cases come along the way.

nkcr commented 3 years ago

@ineiti tests are passing, comments addressed. The proposed trick to use an interface doesn't work. The reason why Calypso tests are not passing is still mysterious to me, but I don't have time to investigate more. I made it pass by not using ChainConfig in config.ts.

nkcr commented 3 years ago

@ineiti defaults added.

ineiti commented 3 years ago

Just remove the UNKNOWN, and I think we're good.

ineiti commented 3 years ago

If you think this is ready, you can move it to R4M. If you think you'll modify it a lot during development of Columbus (which I suppose will happen), you can also teach the student to use a branch of cothority for his development.