Manually implement Debug on Witness

[tcharding]

The current derived debug implementation on Witness prints the content field as an array of integers. We can do better than this by manually implementing Debug.

With this applied Witness is printed as follows: (first line is {:?} and the next is {:#?}):

Using {:?}:

Witness: { indices: 3, indices_start: 8, witnesses: [[0x00], [0x02, 0x03], [0x04, 0x05]] }

Using {:#?}:

Witness: {
    indices: 3,
    indices_start: 8,
    witnesses: [
        [0x00],
        [0x02, 0x03],
        [0x04, 0x05],
     ],
}

[apoelstra]

ACK a44857a

[Kixunil]

Agree with changing the impl but I think it'd be much better to print it as individual elements using debug_list

[tcharding]

Can do, thanks!

[tcharding]

Ok, I responded too quickly. Now I've looked at it, unless I'm missing something, using debug_list does nothing better than the derived version. The point of this PR was that

Witness { content: [71, 48, 68, 2, 32, 60, 181, 14, 251, 92, 74, 154, 167, 253, 54, 154, 182, 244, 178, 38, 219, 153, 244, 79, 156, 97, 11, 91, 80, 188, 66, 243, 67, 166, 170, 64, 19, 2, 32, 26, 247, 145, 84, 46, 238, 108, 27, 17, 112, 94, 136, 149, 204, 90, 220, 54, 69, 137, 16, 220, 145, 170, 220, 175, 183, 106, 100, 120, 162, 155, 159, 1, 33, 2, 66, 232, 17, 230, 111, 209, 126, 154, 110, 78, 247, 114, 118, 108, 102, 141, 110, 5, 149, 202, 29, 127, 5, 131, 20, 139, 196, 96, 181, 117, 251, 253, 0, 0, 0, 0, 72, 0, 0, 0], witness_elements: 2, indices_start: 106 }

Is a bad debugging experience and my proposition was that hex is better than an array of decimal values.

[Kixunil]

@tcharding did you map the values to display as hex? I think list of hex values would be the best, especially using alternate formatting which gets pretty-printed.

[apoelstra]

I also think we should split up the witness fields using indices (though we can also print indices). So the total display would look something like

{
    witnesses: [ [0xab, 0xcd, 0xde], [0x00] ],
    indices: [3],
    indices_start: 1,
}

We should also do a Display impl which does just the witnesses as hex strings, like [abcdde, 00].

[tcharding]

Nice, I can do all that. Thanks

[tcharding]

With some outrageously ugly code I have it looking like this, please ack/nack the layout and I'll clean up the code. I had to manually implement the whole thing because the fmt helpers take mut self so cannot be used togther (debug_struct, and debug_list).

Top one is debug, bottom one is pretty debug.

Witness: { indices: 3, indices_start: 8, witnesses: [ [0x00], [0x02, 0x03], [0x04, 0x05] ]}
Witness: { 
    indices: 3,     
    indices_start: 8,     
    witnesses: [     
        [
            0x00,
        ],
        [
            0x02,
            0x03,
        ],
        [
            0x04,
            0x05,
        ],
     ],
}

[apoelstra]

Looks good to me!

Actually, I think I'd prefer if the pretty one still put all the witness bytes for each witness in one line.

[tcharding]

Actually, I think I'd prefer if the pretty one still put all the witness bytes for each witness in one line.

I thought that myself, so if you've thought it too lets do it.

[tcharding]

Using this format:

Witness: { indices: 3, indices_start: 8, witnesses: [[0x00], [0x02, 0x03], [0x04, 0x05]] }
Witness: { 
    indices: 3,     
    indices_start: 8,     
    witnesses: [     
        [0x00],
        [0x02, 0x03],
        [0x04, 0x05],
     ],
}

Note no space between the square brackets in witnesses on first line, ok with that ?

[tcharding]

The code is pretty ugly still.

[tcharding]

To verify output you can use:

    #[test]
    fn debug_witness() {
        let witness = Witness {
            witness_elements: 3,
            content: append_u32_vec(vec![1u8, 0, 2, 2, 3, 2, 4, 5], &[0, 2, 5]),
            indices_start: 8,
        };

        println!("Print in a line {:?}", witness);
        println!("Print in a line {:#?}", witness);
    }

[apoelstra]

Using this format:

Witness: { indices: 3, indices_start: 8, witnesses: [[0x00], [0x02, 0x03], [0x04, 0x05]] }
Witness: { 
    indices: 3,     
    indices_start: 8,     
    witnesses: [     
        [0x00],
        [0x02, 0x03],
        [0x04, 0x05],
     ],
}

Note no space between the square brackets in witnesses on first line, ok with that ?

I'm happy with this. Would also be happy with any variations on the spacing.

[tcharding]

Rebased on master (to pick up #1927). No other changes.

[tcharding]

Kix's seems out-of-action at the moment, @sanket1729 do you have time to take a look at this one please?

[sanket1729]

On my list for tomorrow

[tcharding]

Legend!

[sanket1729]

tested ACK d45dbef. This would be helpful for debugging downstream.

[apoelstra]

In the interest of getting this in, I'll ACK this. But the allocations are really not necessary and we should file an issue to get rid of them.

[apoelstra]

ACK d45dbef