[message-buffer 11/x] - fix(message-buffer): address PR feedback from 779, update tests #784
Conversation
Add new unit test for reading with cursor, update test setup
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
| if target_size_delta > 0 { | ||
| // allow for delta == 0 in case Rent requirements have changed | ||
| // and additional lamports need to be transferred. | ||
| // the realloc step will be a no-op in this case. |
jayantk
left a comment
jayantk
left a comment
There was a problem hiding this comment.
I'm going to approve this but we should definitely decide on the endianness issue
|
|
||
|
|
||
| let mut cursor = std::io::Cursor::new(&account_info_data[10..]); | ||
| let header_len = cursor.read_u16::<LittleEndian>().unwrap(); |
There was a problem hiding this comment.
i think we agreed that everything would be big endian. it's going to be very confusing if this is little endian while everything else is big endian.
There was a problem hiding this comment.
The MessageBuffer fields are only ever going to be read by the Pythnet validator & Hermes. I'm not sure off the top of my head how to combine BigEndian with zero_copy but i'll leave that as something to look into later on
| let end_offset = header_len + header_end; | ||
| let accumulator_input_data = | ||
| &account_info_data[header_begin as usize..end_offset as usize]; | ||
| header_end = cursor.read_u16::<LittleEndian>().unwrap(); |
There was a problem hiding this comment.
header_end and header_begin are bad names for these two variables because they move to other locations in this loop
| let discriminator = &mut sighash("accounts", "MessageBuffer"); | ||
| let destination = &mut vec![0u8; 10_240 - header_len]; | ||
|
|
||
|
|
There was a problem hiding this comment.
you've copy/pasted a lot of code to initialize message buffers in these tests. I recommend extracting out a helper method that sets everything up for you.
There was a problem hiding this comment.
actually these tests seem to have lots of repeated code blocks in them. I think this isn't ideal -- you should think about how to factor things into helper methods to reduce duplication.
swimricky
changed the title
Add new unit test for reading with cursor, update test setup
Addresses feedback from #779