1 - reading/writing more than 32 bytes into the TendConfig.extraData field
#43
Conversation
….extraData` field
mcclurejt
changed the title
TendConfig.extraData fieldTendConfig.extraData field
|
|
||
| /// @dev Tests that `TendConfig.extraData` is fully loaded even when the | ||
| /// content is greater than a single storage slot. | ||
| function test_extraData_large() external { |
There was a problem hiding this comment.
You'll probably want to add tests for cases that assert when extraData is "", < bytes32 bytes, and > bytes32 bytes.
I re-ran the original test with a single char bytes input and the output wasn't matching the input.
Ensure you include assertions on minOutput, minVaultSharePrice, and positionClosureLimit when populated with non-zero values as well.
There was a problem hiding this comment.
I re-ran the original test with a single char bytes input and the output wasn't matching the input.
I couldn't reproduce this, can you show the example. (but agree with adding different test cases for different extreme cases of slots occupied.)
mcclurejt
changed the title
TendConfig.extraData fieldTendConfig.extraData field
| // Pointer to the actual data. | ||
| let extraDataData := add(extraData, 0x20) | ||
|
|
||
| // Store the length in the `extraDataSlot`. | ||
| tstore(extraDataSlot, extraDataLength) | ||
|
|
||
| // Store the data in subsequent slots. | ||
| for { | ||
| let i := 0 | ||
| } lt(i, extraDataLength) { | ||
| i := add(i, 32) | ||
| } { | ||
| tstore( | ||
| add(extraDataSlot, add(div(i, 32), 1)), | ||
| mload(add(extraDataData, i)) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Nit: We're mixing and matching hex and decimals here. It's up to you whether or not you want to fix this, but I generally prefer all hex in inline assembly.
There was a problem hiding this comment.
I got a concern here which involves some low level memory layout that I'm very certain about the details: does the compiler ensure that the extraDataData always gets padded into 32 bytes for the last few remaining words if it's short of 32?
Here the mload is always loading 32 in each iteration, I'm thinking if it has risks of reading unwanted data for the last piece if the memory layout didn't get padded to a full 32? Maybe some quick tests of extraDataLength not being exactly multiples of 32 can help answering it.
Only the first 32 bytes of
extraDataare read/written fromTendConfig.This PR adds logic to handle more than 32 bytes for
TendConfig.extraData.issue