feat(byte_array): add ByteArray::span and Span::len

[giladchase]

No description provided.

[reviewable-StarkWare]

This change is 

[giladchase]

• feat(byte_array): add at and index to ByteSpan #8427
• feat(byte_array): add slice #8417
• feat(byte_array): add ByteSpan::to_byte_array #8416
• refactor(byte_array): extract ByteArray::append logic #8486
• feat(byte_array): add ByteArray::span and Span::len #8329 👈 (View in Graphite)
• refactor(test): remove fixed-size bytearray testutils #8343
• refactor(test): remove compare_spans util #8342
• refactor(test): remove compare_byte_array util and rework tests #8336
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[TomerStarkware]

@TomerStarkware reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @orizi)

---

corelib/src/byte_array.cairo line 611 at r1 (raw file):

    fn len(self: @Span) -> usize {
        let raw_data_bytes = self.raw_data.len() * BYTES_IN_BYTES31;
        raw_data_bytes - *self.first_char_start_offset + *self.last_char_end_offset

casting everything to felt252 and then casting back to size would be more performant

Code quote:

raw_data_bytes - *self.first_char_start_offset + *self.last_char_end_offset

[TomerStarkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @orizi)

---

corelib/src/byte_array.cairo line 611 at r1 (raw file):

Previously, TomerStarkware wrote…

casting everything to felt252 and then casting back to size would be more performant

this is wrong

[orizi]

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)

---

corelib/src/byte_array.cairo line 593 at r2 (raw file):

/// `Span` implements the `Copy` and the `Drop` traits.
#[derive(Copy, Drop)]
pub struct Span {

probably.

Suggestion:

pub struct ByteSpan {

[giladchase]

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 593 at r2 (raw file):

Previously, orizi wrote…

probably.

Done.

---

corelib/src/byte_array.cairo line 598 at r3 (raw file):

    /// slices.
    /// Value should be in the range [0, 30].
    pub(crate) first_char_start_offset: usize,

Replace here and other with BoundedInt

Code quote:

    /// Value should be in the range [0, 30].
    pub(crate) first_char_start_offset: usize,

[giladchase]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 64 at r13 (raw file):

Previously, giladchase wrote…

Done.
Note that once we make ByteArray use this typedef we might want to pub(crate) this typdef, since it will be used in a pub(crate) field of ByteArray (pending_word_len).

The Done is a lie.
Now it's done.

[orizi]

Reviewable status: 0 of 2 files reviewed, 5 unresolved discussions (waiting on @TomerStarkware)

---

corelib/src/byte_array.cairo line 634 at r15 (raw file):

    fn len(self: @ByteSpan) -> usize {
        let data_bytes = self.data.len() * BYTES_IN_BYTES31;
        data_bytes + upcast(self.remainder_len) - upcast(self.first_char_start_offset)

Suggestion:

        downcast(bounded_int::sub(
            bounded_int::add(
                bounded_int::mul(self.data.len(), BYTES_IN_BYTES31),
                self.remainder_len),
            ),
            self.first_char_start_offset,
        )).unwrap()

---

corelib/src/byte_array.cairo line 651 at r15 (raw file):

    /// ```
    fn is_empty(self: @ByteSpan) -> bool {
        self.data.len() == 0 && *self.remainder_len == 0

can you check both: (in any case - opposite order probably marginally better)

Suggestion:

        *self.remainder_len == 0 && self.data.len() == 0
        
        |||
        
        *self.remainder_len == 0 && self.data.is_empty()

---

corelib/src/byte_array.cairo line 655 at r15 (raw file):

}

pub trait ToByteSpanTrait<C> {

doc

[giladchase]

Reviewable status: 0 of 2 files reviewed, 5 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 651 at r15 (raw file):

*self.remainder_len == 0 && self.data.len() == 0

wins, the is_empty variant is a tad slower than this one, so i'm picking the len variant unless you still prefer the is_empty option.

Both of yr suggestions are faster than current code due to the order switch.

---

corelib/src/byte_array.cairo line 634 at r15 (raw file):

    fn len(self: @ByteSpan) -> usize {
        let data_bytes = self.data.len() * BYTES_IN_BYTES31;
        data_bytes + upcast(self.remainder_len) - upcast(self.first_char_start_offset)

Done.
I added the operator impls in a helpers mod like in bytes_31.cairo.
Should i define U32ByB31 in bytes_31? Wasn't sure if it's interesting enough

[orizi]

@orizi reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @giladchase and @TomerStarkware)

---

corelib/src/byte_array.cairo line 65 at r17 (raw file):

/// The number of bytes in [`ByteArray::pending_word`].
type WordBytes = BoundedInt<0, { BYTES_IN_BYTES31_MINUS_ONE.into() }>;
type BYTES_IN_BYTES31_TYPED = UnitInt<{ BYTES_IN_BYTES31.into() }>;

this is a type - not a const.

---

corelib/src/byte_array.cairo line 707 at r17 (raw file):

    }
}
use helpers::{B30AddU32ByB31, B30AddU32ByB31SubB30, U32ByB31};

if you want these just for a single function - add the use within it.
better yet - just add the len calculation as a function in the helper.
(and remove the pub from the impls)

Code quote:

use helpers::{B30AddU32ByB31, B30AddU32ByB31SubB30, U32ByB31};

[giladchase]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 655 at r15 (raw file):

Previously, orizi wrote…

doc

Done.

---

corelib/src/byte_array.cairo line 65 at r17 (raw file):

Previously, orizi wrote…

this is a type - not a const.

Done.

---

corelib/src/byte_array.cairo line 707 at r17 (raw file):

Previously, orizi wrote…

if you want these just for a single function - add the use within it.
better yet - just add the len calculation as a function in the helper.
(and remove the pub from the impls)

This OK? Or also add push the downcast into it?

[orizi]

@orizi reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @giladchase and @TomerStarkware)

---

corelib/src/byte_array.cairo line 707 at r17 (raw file):

Previously, giladchase wrote…

This OK? Or also add push the downcast into it?

downcast as well IMO - would be better encapsulation.

---

corelib/src/byte_array.cairo line 692 at r18 (raw file):

    impl U32ByB31 of MulHelper<u32, BytesInBytes31Typed> {
        type Result = BoundedInt<0, { U32_MAX_TIMES_B31 }>;

Suggestion:

        type Result = BoundedInt<0, U32_MAX_TIMES_B31>;

[giladchase]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 707 at r17 (raw file):

Previously, orizi wrote…

downcast as well IMO - would be better encapsulation.

Done.

[orizi]

@orizi reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @giladchase and @TomerStarkware)

---

corelib/src/byte_array.cairo line 715 at r20 (raw file):

    }
}
use helpers::calc_bytespan_len;

no need for the use IMO.

Code quote:

use helpers::calc_bytespan_len;

[giladchase]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 715 at r20 (raw file):

Previously, orizi wrote…

no need for the use IMO.

Right, replaced with a qualified path.

---

corelib/src/byte_array.cairo line 692 at r18 (raw file):

    impl U32ByB31 of MulHelper<u32, BytesInBytes31Typed> {
        type Result = BoundedInt<0, { U32_MAX_TIMES_B31 }>;

Done.

[orizi]

@orizi reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @TomerStarkware)

---

corelib/src/byte_array.cairo line 650 at r21 (raw file):

    /// ```
    fn is_empty(self: ByteSpan) -> bool {
        self.remainder_len == 0 && self.data.len() == 0

doc why this is enough.

Code quote:

self.remainder_len == 0

[giladchase]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 650 at r21 (raw file):

Previously, orizi wrote…

doc why this is enough.

Done.

[orizi]

@orizi reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @giladchase and @TomerStarkware)

---

corelib/src/byte_array.cairo line 650 at r21 (raw file):

Previously, giladchase wrote…

Done.

do the implementation - not the function.

[giladchase]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 650 at r21 (raw file):

Previously, orizi wrote…

do the implementation - not the function.

Done.

[orizi]

@orizi reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)

---

corelib/src/byte_array.cairo line 650 at r23 (raw file):

    /// ```
    fn is_empty(self: ByteSpan) -> bool {
        // No partial bytes31-sized words in `remainder_word`, nor any full words in `data`.

i meant that you need to doc why self.remainder_len == 0 is enough.
since you COULD have had something like "abcd".get(1..)

Code quote:

        // No partial bytes31-sized words in `remainder_word`, nor any full words in `data`.

[giladchase]

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @orizi and @TomerStarkware)

---

corelib/src/byte_array.cairo line 650 at r23 (raw file):

Previously, orizi wrote…

i meant that you need to doc why self.remainder_len == 0 is enough.
since you COULD have had something like "abcd".get(1..)

Is this what you meant? I'm not exactly sure what you're aiming for, i'm assuming you meant that small words are entirely located in the remainder? (i tried to address this in the comment now)

[orizi]

@orizi reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @giladchase and @TomerStarkware)

---

corelib/src/byte_array.cairo line 650 at r23 (raw file):

Previously, giladchase wrote…

Is this what you meant? I'm not exactly sure what you're aiming for, i'm assuming you meant that small words are entirely located in the remainder? (i tried to address this in the comment now)

so not enough - can't self.remainder_len == self.first_char_start_offset. while self.remainder_len isn't 0?
or do you specifically prevent this case?

that is the doc i am looking for.