stratum-mining / stratum

stratum
https://stratumprotocol.org
Other
207 stars 122 forks source link

framing_sv2 set wrong header channel bit #1147

Closed Fi3 closed 2 weeks ago

Fi3 commented 2 weeks ago

This function in framing_sv2::framing:

fn update_extension_type(extension_type: u16, channel_msg: bool) -> u16 {
    if channel_msg {
        let mask = 0b1000_0000_0000_0000;
        extension_type | mask
    } else {
        let mask = 0b0111_1111_1111_1111;
        extension_type & mask
    }
}

Set the bit 0 as channel bit. But spec say that have to be bit 15, 0-indexed, https://github.com/stratum-mining/sv2-spec/blob/main/03-Protocol-Overview.md#32-framing.

The below function should fix the issue

fn update_extension_type(extension_type: u16, channel_msg: bool) -> u16 {
    if channel_msg {
        let mask = 0b0000_0000_0000_0001;
        extension_type | mask
    } else {
        let mask = 0b1111_1111_1111_1110;
        extension_type & mask
    }
}
xyephy commented 2 weeks ago

Hi @Fi3 can I give this a try?

Fi3 commented 2 weeks ago

sure @xyephy

plebhash commented 2 weeks ago

@xyephy please work on top of this https://github.com/stratum-mining/stratum/pull/1049

xyephy commented 2 weeks ago

@xyephy please work on top of this #1049

Noted.

jakubtrnka commented 2 weeks ago

In my opinion. the channel-bit should be the most significant bit in the extension_type field. That means in the serialized form it should be the 15'th bit counted from zero. It's because we use little-endian representation.

This means, I probably disagree with the proposed change.

Fi3 commented 2 weeks ago

@jakubtrnka is right we can close this issue 0b1000_0000_0000_0000 already set the most significant bit. @xyephy