byteArrayFromString produces incorrect result for short string

[DelevoXDG]

Describe the bug

The algorithm, used both in byteArray.ts and typedData.ts seems to produce incorrect results for short strings of length <31.

It ensures at least one value (0x00) is present in ByteArray's data, contrary to the docs, where in such case data should be empty.

To Reproduce
Run the following test, based on the example from the docs:

  test('convert string to ByteArray', () => {
    expect(byteArray.byteArrayFromString('hello')).toEqual({
      data: [],
      pending_word: '0x68656c6c6f',
      pending_word_len: 5,
    });
  });

Expected behavior
Test passes

Actual behavior
Test fails, actual ByteArray is:

data: ['0x00'],
pending_word: '0x68656c6c6f',
pending_word_len: 5,

Desktop (please complete the following information):

• Starknet.js version fdcc7fb

[tabaktoni]

Thank you for reporting, we will look into it. @PhilippeR26

[PhilippeR26]

Hello,
This documentation is not conform to the real behavior of Starknet.
When Starknet serializes a ByteArray including less than 31 characters, the result is

[
  '0x1',
  '0x0',
  '0x54616b6520636172652e205a6f7267206973206261636b',
  '0x17'
]

'0x1','0x0', means an array of one element (value 0).

Starknet.js will continue to follow the real behavior of the Starknet network.

[DelevoXDG]

Thanks for your reply.

Could you provide further clarification on how you verified that current behaviour is the correct one?

It's important to note that the behavior described in the docs is not a typo; it's expected, as indicated in the tests within the cairo repo.

[PhilippeR26]

I just deployed in Sepolia the Starknet.js test contract at : 0x660edd51a76b970cc8b843e5e0560e48afa2bb73e2530ab9a7425b17c1366b2
If you launch this :

const resp4= await stringContract.call("proceed_string",["Take care."],{parseResponse:false});

The response is :

0x1,0x0,0x54616b6520636172652e205a6f7267206973206261636b,0x17

Cairo source code : https://github.com/starknet-io/starknet.js/blob/develop/__mocks__/cairo/cairo240/string.cairo

[amanusk]

The issue seems to be with the way data is serialized and sent when calling proceed_string.

Calling the following function in Cairo/snforge returns the data array as explained in the doc

fn proceed_array(self: @ContractState, mess: ByteArray) -> Array<felt252> {
            let mut res = mess;
            let add: ByteArray = " Zorg is back";
            res.append(@add);

            let mut serialized_string: Array<felt252> = Default::default();
            res.serialize(ref serialized_string);
            serialized_string
        }

println will return [0, 1614488889802844162655565249641883848014914411, 19]

But, calling the function in the test, from sn.js returns an array with "0x0" as a first element.

To confirm this, a function w/o any appending will return the correct structure.

        fn default_string(self: @ContractState) -> ByteArray {
            let res: ByteArray = "Zorg is back";
            res
        }

calling it from sn.js, results in the following result

  let resp4 = await myTestContract.call("default_string", [], { parseResponse: false });
  console.log("Default sting");
  console.log(resp4);

will result in [ '0x0', '0x5a6f7267206973206261636b', '0xc' ], as expected and described in docs.

TL;DR, ByteArray should be serialized as explained in the docs, but sn.js is serializing them incorrectly when sending them to a contract

[PhilippeR26]

But, calling the function in the test, from sn.js returns an array with "0x0" as a first element.

I think I understood, and I will correct this.

Shouldn't it be somewhere after the SDK request a check of the consistency of the serialized ByteArray?
I made some tests :

• If pending_word_len is not the one expect, ans is too low, the contract reverts, with 'Option::unwrap failed.').
• If pending_word_len is not the one expect, and is too high, there is no error, and the returned processed ByteArray has still an error.