chore!: ToRadixLE -> ToRadixBE in Brillig and AVM

[dbanks12]

No description provided.

[dbanks12]

• chore!: ToRadixLE -> ToRadixBE in Brillig and AVM #9340 👈
• chore!: use Brillig opcode when possible for less-than operations on fields #9416
• master

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

Join @dbanks12 and the rest of your teammates on Graphite

[github-actions]

Changes to public function bytecode sizes

Generated at commit: 8b214445d0f00c13c601f8a4e3dee48f1cc7ca0c, compared to commit: f4ed55b264ff92979e6e655508b8f8fac826086e

🧾 Summary (100% most significant diffs)

Program	Bytecode size in bytes (+/-)	%
AvmTest::to_radix_le	+144 ❌	+59.02%
AvmTest::keccak_hash	+144 ❌	+5.35%
AvmTest::sha256_hash	+144 ❌	+3.81%
CardGame::on_game_joined	+144 ❌	+3.07%
CardGame::on_card_played	+144 ❌	+2.58%
CardGame::start_game	+144 ❌	+2.16%
CardGame::on_cards_claimed	+144 ❌	+2.06%
CardGame::public_dispatch	+144 ❌	+0.92%
AvmTest::bulk_testing	+144 ❌	+0.68%
AvmTest::public_dispatch	+184 ❌	+0.33%
TokenBridge::public_dispatch	-20 ✅	-0.10%
Test::public_dispatch	-20 ✅	-0.11%
TokenBridge::claim_public	-20 ✅	-0.17%
Test::consume_mint_public_message	-20 ✅	-0.20%
Token::public_dispatch	-164 ✅	-0.51%
NFT::public_dispatch	-164 ✅	-0.64%
NFT::_finalize_transfer_to_private_unsafe	-164 ✅	-2.61%
NFT::finalize_transfer_to_private	-164 ✅	-2.62%
Token::complete_refund	-164 ✅	-2.80%

---

Full diff report 👇

Program	Bytecode size in bytes (+/-)	%
AvmTest::to_radix_le	388 (+144)	+59.02%
AvmTest::keccak_hash	2,836 (+144)	+5.35%
AvmTest::sha256_hash	3,920 (+144)	+3.81%
CardGame::on_game_joined	4,833 (+144)	+3.07%
CardGame::on_card_played	5,718 (+144)	+2.58%
CardGame::start_game	6,821 (+144)	+2.16%
CardGame::on_cards_claimed	7,133 (+144)	+2.06%
CardGame::public_dispatch	15,848 (+144)	+0.92%
AvmTest::bulk_testing	21,411 (+144)	+0.68%
AvmTest::public_dispatch	55,743 (+184)	+0.33%
TokenBridge::public_dispatch	20,983 (-20)	-0.10%
Test::public_dispatch	18,933 (-20)	-0.11%
TokenBridge::claim_public	11,478 (-20)	-0.17%
Test::consume_mint_public_message	10,146 (-20)	-0.20%
Token::public_dispatch	31,961 (-164)	-0.51%
NFT::public_dispatch	25,298 (-164)	-0.64%
NFT::_finalize_transfer_to_private_unsafe	6,128 (-164)	-2.61%
NFT::finalize_transfer_to_private	6,097 (-164)	-2.62%
Token::complete_refund	5,694 (-164)	-2.80%

[fcarreiro]

I'm surprised this doesn't give us bytecode gains. @sirasistant I thought Brillig was needing some conversions LE -> BE because it was expecting to do ToRadixBE?

[dbanks12]

I'm surprised this doesn't give us bytecode gains. @sirasistant I thought Brillig was needing some conversions LE -> BE because it was expecting to do ToRadixBE?

I thought the same @fcarreiro! @sirasistant's response was "should not save much in bytecode size, but it'd have better runtime"

[sirasistant]

I'm surprised this doesn't give us bytecode gains

Reversals are deduplicated via a procedure call so it shouldn't be noticeable. However I think we might be using little endianness more than I thought given the measurement changes

[sirasistant]

Ah I see, card game uses to_le_bytes for packing and

fn compute_lt(x: Field, y: Field, num_bytes: u32) -> bool {
    let x_bytes: [u8; 32] = x.to_le_bytes();
    let y_bytes: [u8; 32] = y.to_le_bytes();
    let mut x_is_lt = false;
    let mut done = false;
    for i in 0..num_bytes {
        if (!done) {
            let x_byte = x_bytes[num_bytes - 1 - i];
            let y_byte = y_bytes[num_bytes - 1 - i];
            let bytes_match = x_byte == y_byte;
            if !bytes_match {
                x_is_lt = x_byte < y_byte;
                done = true;
            }
        }
    }
    x_is_lt
}

Field comparisons in the stdlib are using little endian (probably because before this PR little endian was cheaper)

[sirasistant]

Maybe we can switch the compute_lt and the card game to use be and see what the results are

[fcarreiro]

Ah I see, card game uses to_le_bytes for packing and

fn compute_lt(x: Field, y: Field, num_bytes: u32) -> bool {
    let x_bytes: [u8; 32] = x.to_le_bytes();
    let y_bytes: [u8; 32] = y.to_le_bytes();
    let mut x_is_lt = false;
    let mut done = false;
    for i in 0..num_bytes {
        if (!done) {
            let x_byte = x_bytes[num_bytes - 1 - i];
            let y_byte = y_bytes[num_bytes - 1 - i];
            let bytes_match = x_byte == y_byte;
            if !bytes_match {
                x_is_lt = x_byte < y_byte;
                done = true;
            }
        }
    }
    x_is_lt
}

Field comparisons in the stdlib are using little endian (probably because before this PR little endian was cheaper)

Hmm, does this mean that we should drop this PR? or change the stdlib?

[dbanks12]

Does seem to reduce gas costs for some e2e tests by a bit! Not a massive improvement. TokenBridge:claim_public goes from 3,970,000 down to 3,930,000.

[dbanks12]

Replaced by #9471