Make visualizing forc log output pretty

[SilentCicero]

• Right now log data is pretty visually hard to parse with Forc.
• It would be nice to simplify this or have a simplified view or option available

For this program (as an exampe):

script;

fn log_b256(value: b256) {
    asm(r1: value, r2: 32) {
        logd zero zero r1 r2;
    }
}

fn main() {
    let k:b256 = 0xef86afa9696cf0dc6385e2c407a6e159a1103cefb7e2ae0636fb33d3cb2a9e4a;

    log_b256(k);
}

The forc run return looks like this:

[LogData { id: 0000000000000000000000000000000000000000000000000000000000000000, ra: 0, rb: 0, ptr: 10472, len: 32, digest: be240c408b034a8f3e67813d85c05d5379e02c17d4012440a4bf73450aa8935d, data: [239, 134, 175, 169, 105, 108, 240, 220, 99, 133, 226, 196, 7, 166, 225, 89, 161, 16, 60, 239, 183, 226, 174, 6, 54, 251, 51, 211, 203, 42, 158, 74], pc: 10416, is: 10352 }, Return { id: 0000000000000000000000000000000000000000000000000000000000000000, val: 0, pc: 10420, is: 10352 }, ScriptResult { result: InstructionResult { reason: RESERV00, instruction: Instruction { op: 0, ra: 0, rb: 0, rc: 0, rd: 0, imm06: 0, imm12: 0, imm18: 0, imm24: 0 } }, gas_used: 424 }]

Could look like this:

Logs Returned by Execution:

LogData:
    Register A: 0
    Register B: 0
    Data: 0xef86afa9696cf0dc6385e2c407a6e159a1103cefb7e2ae0636fb33d3cb2a9e4a

Having some kind of easy to parse clean version would be nice. If I want all the other program details I could just maybe flag an option for it.

[kayagokalp]

To clarify a little bit, as I understand forc run needs to return in a more human-readable format. Does this include prunings of the fields in the output? (If so which fields will be staying?) or we are going to just format and keep every field?

[adlerjohn]

1. Pretty-print the JSON
2. Convert the data field of receipts to hex. I think all receipts with a data field can have this conversion done, so a simple replacement of all data fields in the JSON should suffice to resolve this.

[kayagokalp]

Maybe I can summarize my current way of thinking/doing this and get a little feedback, if possible :)

I am currently working towards getting the data field converted into hex, I am looking at (and testing) two options, converting the receipt to JSON with serde_json and converting the data field to hex. This results in outputs like the following

Although the data field started to look better, serde_json converts other fields (such as digest and id) into a more verbose version. I can do the same formatting I am doing for the data field to those fields as well.

I guess another option is not converting into JSON and keeping the current way of printing (which uses the debug trait) and doing a string substitution there (with regex maybe).

I am not sure which way is better to continue with (leaning towards the first option but I may be missing out on a better and different option).

[adlerjohn]

Ah right. id and digest are also hashes and so should also be displayed in hex. I'm not sure either about which approach is best. cc @Voxelot for help on what the client emits.

[kayagokalp]

Looking at the receipt.rs I think the following fields should be converted into hex in the output:

• All id fields
• All to fields
• All asset_id fields
• All digest fields
• All data fields

I converted these fields to hex in the default output and added a flag --verbose to keep printing as we do now to address the following part:

If I want all the other program details I could just maybe flag an option for it.

Currently, I am not happy with the code quality of my implementation of the formatting section and now working on refactoring that part. Current outputs with different flags are as the following:

• cargo run --bin forc -- run --path my_test

  {"LogData":{"data":"ef86afa9696cf0dc6385e2c407a6e159a1103cefb7e2ae0636fb33d3cb2a9e4a","digest":"be240c408b034a8f3e67813d85c05d5379e02c17d4012440a4bf73450aa8935d","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10352,"len":32,"pc":10416,"ptr":10488,"ra":0,"rb":0}}
  {"Return":{"id":"0000000000000000000000000000000000000000000000000000000000000000","is":10352,"pc":10424,"val":0}}
  {"ScriptResult":{"gas_used":435,"result":"Success"}}

• cargo run --bin forc -- run --path my_test --pretty-print

  {
  "LogData": {
  "data": "ef86afa9696cf0dc6385e2c407a6e159a1103cefb7e2ae0636fb33d3cb2a9e4a",
  "digest": "be240c408b034a8f3e67813d85c05d5379e02c17d4012440a4bf73450aa8935d",
  "id": "0000000000000000000000000000000000000000000000000000000000000000",
  "is": 10352,
  "len": 32,
  "pc": 10416,
  "ptr": 10488,
  "ra": 0,
  "rb": 0
  }
  }
  {
  "Return": {
  "id": "0000000000000000000000000000000000000000000000000000000000000000",
  "is": 10352,
  "pc": 10424,
  "val": 0
  }
  }
  {
  "ScriptResult": {
  "gas_used": 435,
  "result": "Success"
  }
  }

• cargo run --bin forc -- run --path my_test --verbose

  [LogData { id: 0000000000000000000000000000000000000000000000000000000000000000, ra: 0, rb: 0, ptr: 10488, len: 32, digest: be240c408b034a8f3e67813d85c05d5379e02c17d4012440a4bf73450aa8935d, data: [239, 134, 175, 169, 105, 108, 240, 220, 99, 133, 226, 196, 7, 166, 225, 89, 161, 16, 60, 239, 183, 226, 174, 6, 54, 251, 51, 211, 203, 42, 158, 74], pc: 10416, is: 10352 }, Return { id: 0000000000000000000000000000000000000000000000000000000000000000, val: 0, pc: 10424, is: 10352 }, ScriptResult { result: Success, gas_used: 435 }]

• cargo run --bin forc -- run --path my_test --verbose --pretty-print

  [
  LogData {
      id: 0x0000000000000000000000000000000000000000000000000000000000000000,
      ra: 0,
      rb: 0,
      ptr: 10488,
      len: 32,
      digest: 0xbe240c408b034a8f3e67813d85c05d5379e02c17d4012440a4bf73450aa8935d,
      data: [
          239,
          134,
          175,
          169,
          105,
          108,
          240,
          220,
          99,
          133,
          226,
          196,
          7,
          166,
          225,
          89,
          161,
          16,
          60,
          239,
          183,
          226,
          174,
          6,
          54,
          251,
          51,
          211,
          203,
          42,
          158,
          74,
      ],
      pc: 10416,
      is: 10352,
  },
  Return {
      id: 0x0000000000000000000000000000000000000000000000000000000000000000,
      val: 0,
      pc: 10424,
      is: 10352,
  },
  ScriptResult {
      result: Success,
      gas_used: 435,
  },
  ]

Although the data field started to look better, serde_json converts other fields (such as digest and id) into a more verbose version. I can do the same formatting I am doing for the data field to those fields as well.

Currently, I went ahead with the mentioned approach but depending on any input from @Voxelot, I can implement this differently. Also not sure about whether we want to keep the --verbose flag.

[Voxelot]

While we can make various enhancements to upstream crates to improve the rendering here and there, I think there's a good argument for forc to have its own log rendering system that has complete control over the format it outputs to users. For example, what may be a good way to serialize types for an API (JSON) may not be the most friendly format for CLI usage.

That said, I can't think of a case where we'd want to serialize a b256 as an [int], which is the default serde behavior. Almost all areas of our stack would be simplified if we made a custom serialize/deserialize impl for key! types that used hex serialization. https://github.com/FuelLabs/fuel-types/blob/master/src/types.rs#L30

[Voxelot]

logged an issue here: https://github.com/FuelLabs/fuel-types/issues/31

[adlerjohn]

How hard would it be to implement hex serialization in fuel-types? If not to hard, then maybe it's better to just change it there once.

[Voxelot]

Pretty simple, it's a good first issue if someone wants to pick it up.

However, without its own receipt formatting/printing adapter, forc would never be able to customize the output enough to end up with what Nick originally posted.

Logs Returned by Execution:

LogData:
    Register A: 0
    Register B: 0
    Data: 0xef86afa9696cf0dc6385e2c407a6e159a1103cefb7e2ae0636fb33d3cb2a9e4a

[adlerjohn]

Okay yeah that extra formatting isn't really needed at this time. The only important thing is hex-formatting hashes and indenting the JSON.

[adlerjohn]

@kayagokalp do you think you could tackle https://github.com/FuelLabs/fuel-types/issues/31 as a prerequisite to resolving this issue, instead of manually choosing specific fields to hex-format as per your current approach?

[kayagokalp]

Yep, getting started to work on that one 👍

[mitchmindtree]

Related #884, w.r.t. more flexibility around logging in general.

[vlopes11]

This doesn't look like a serialization problem. We might cause some long-term trouble if we custom the base type to serve some specific display use.

It looks to me that we need to implement a customized Display to these types, without touching sensible aspects such as serialization. That way, we have absolute freedom to manipulate the format as we wish, without ever having to worry with any potential side-effects that might be caused when we interfere with serialization strategies. Note that we might even use customized ad-hoc serde consumers in the display implementation - we will achieve the same effect.

If we really want to stick to the strategy of customizing the serde serialization, we need to use custom with attributes. For reference: serde-hex

Serde, by design, will force us to perform the undesirable update of the raw types in order to affect its consumers. This alternative, however, is less worse, because we still use proc macros and will (likely) still be compatible with future version upgrades.

cc @kayagokalp

[Voxelot]

This was primarily motivated by the fact that whenever people want to JSON serialize fuel-types, they use the serde_json adaptor. The ABI specifies that the receipts should be JSON serializable, and that they should use hex encoding for all the byte arrays and numeric types: https://github.com/FuelLabs/fuel-specs/blob/master/specs/protocol/abi.md#panic-receipt

Relying only on the Display trait for rendering these types will make it very difficult for downstream consumers of these types to serialize them to JSON in a usable way. Leading to many wrappers or new types and other clunky workarounds.

The reason why I suggested using a custom impl of serialize/deseralize is so that we have consistent logic for converting our types to and from hex. Otherwise there could be subtle and different behaviors between deserialize and from_str if we relied on serde-hex instead of reusing our own logic.

The downside is, hex string serialization is not the most efficient serialization format in all use-cases (esp db storage in fuel-core for example). However, the alternative is customizing our own version of serde_json to follow our formatting conventions and then educating users to use our custom adapter instead.

To me, it seems like simplifying the most common serialization usecases that users will most frequently encounter and then making special exceptions and workarounds in places like db storage is preferable to adhering to strict principles that inconvenience the majority of users.

[Voxelot]

Put another way, here are all the places we would use serde with our types:

• GraphQL API JSON
• JSON chainspec configuration
• Logging
• Database storage

3/4 cases, which are all the user-facing ones, we would want to serialize receipts according to the ABI spec. The only case where we wouldn't want serde to use hex-string based serialization is in the database, which is non-user facing and where we can internally optimize as needed. We'd be able to simplify a lot of code involving fuel-types if it supported hex-based serialization by default.

[kayagokalp]

After these comments, I feel like we should continue with custom hex serialization implementation but just to work around this issue we can always go down the Display or Debug path too.

[Voxelot]

had a chat with the client team and @vlopes11 agreed that we can go ahead and make serde the defacto serialization method for pretty user-facing stuff, since for networking we'll likely use protobufs and on the db we can use something else like borsh or just manually serialize these types optimally for the db ourselves.

[mitchmindtree]

@kayagokalp you might already be aware of this, but just in case:

Keep in mind that if we do need to format just one particular field in a way that differs from the default serialization, that serde provides nice ways of allowing custom field serialization/deserialization even if that type has an existing Serialize/Deserialize implementation that differs to what we want.

There are some great docs on how to achieve this here.

[kayagokalp]

@mitchmindtree , thanks for the tip! Currently the default serialization of key! types are merged (fuel-types#34 and released. Once an update to the fuel-tx is ready (which switches fuel-types to v0.5.0 and possibly adds a serialize(with..) annotation to Vec<u8> fields (data)) we will be able to use it directly. But I wasn't aware of this and it seems super helpful.

[kayagokalp]

This is unblocked after https://github.com/FuelLabs/sway/pull/1683, opening up the PR 🎉