Update basic order flow to avoid allocating unnecessary memory #45
Conversation
|
@0age, here is at least one more edit that is needed: This head of the // Derive pointer to start of OrderFulfilled event data.
let eventDataPtr :=
add(
OrderFulfilled_baseOffset,
shl(
OneWordShift,
calldataload(
BasicOrder_additionalRecipients_length_cdPtr // <--- need to change this as well
)
)
) |
0age
force-pushed
the
wip-basic-order-memory-optimization
branch
from
ac61dbe
to
4d5138d
Compare
|
made the suggested change @Saw-mon-and-Natalie — still seeing failures however 🙃 |
0age
changed the title
…used in to copy it from.
| mstore(add(eventDataPtr, 0x80), 1) | ||
|
|
||
| // Write itemType to the SpentItem struct. | ||
| mstore(add(eventDataPtr, 0xa0), offeredItemType) | ||
|
|
||
| // Copy calldata region with (offerToken, offerIdentifier, | ||
| // offerAmount) from OrderParameters to (token, identifier, | ||
| // amount) in SpentItem struct. | ||
| calldatacopy( | ||
| add(eventDataPtr, 0xc0), |
There was a problem hiding this comment.
TODO: define named constants for these 3 new offsets.
| // Set a length of 1 for the offer array. | ||
| mstore(add(eventDataPtr, 0x80), 1) | ||
|
|
||
| // Write itemType to the SpentItem struct. | ||
| mstore(add(eventDataPtr, 0xa0), offeredItemType) | ||
|
|
||
| // Copy calldata region with (offerToken, offerIdentifier, | ||
| // offerAmount) from OrderParameters to (token, identifier, | ||
| // amount) in SpentItem struct. | ||
| calldatacopy( | ||
| add(eventDataPtr, 0xc0), | ||
| BasicOrder_offerToken_cdPtr, | ||
| ThreeWords | ||
| ) |
There was a problem hiding this comment.
I moved these down here, since it would make it more clear what we are doing in memory and also would require less calculation of the pointer.
and automatically the offset is also corrected.
| shl( | ||
| OneWordShift, | ||
| calldataload( | ||
| BasicOrder_additionalRecipients_length_cdPtr |
There was a problem hiding this comment.
this was wrong, we needed to use:
BasicOrder_totalOriginalAdditionalRecipients_cdPtr
|
Also for restricted orders pre and post hook we can reuse the same memory region for spent and received items without copying that region again. Just need to rewrite the memory before the region and make sure to allocate 3 word slots after that region for the order hashes and the |
| uint256 additionalRecipientsLength = CalldataPointer.wrap( | ||
| BasicOrder_additionalRecipients_length_cdPtr | ||
| ).readUint256(); | ||
|
|
||
| // Retrieve the length of additional recipients. |
There was a problem hiding this comment.
| // Retrieve the length of additional recipients. | |
| // Retrieve the total original recipients length. |
There was a problem hiding this comment.
can be fixed in a different PR or even this one.
0age
changed the title
|
cc @Saw-mon-and-Natalie to fix the TODOs and comments in a follow-up PR |
| // Set a length of 1 for the offer array. | ||
| mstore(add(eventDataPtr, 0x80), 1) | ||
|
|
||
| // Write itemType to the SpentItem struct. | ||
| mstore(add(eventDataPtr, 0xa0), offeredItemType) | ||
|
|
||
| // Copy calldata region with (offerToken, offerIdentifier, | ||
| // offerAmount) from OrderParameters to (token, identifier, | ||
| // amount) in SpentItem struct. | ||
| calldatacopy( | ||
| add(eventDataPtr, 0xc0), | ||
| BasicOrder_offerToken_cdPtr, | ||
| ThreeWords | ||
| ) |
There was a problem hiding this comment.
note for myself: these are all shifted by 0x80 compared to the previous version because old eventConsiderationArrPtr used OrderFulfilled_offer_length_baseOffset = 0x200, whereas this eventDataPtr only uses OrderFulfilled_baseOffset = 0x180. The diff is the 0x80.
Then we write to 0x80+0, 0x80+0x20, 0x80+0x40
No description provided.