Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BasicOrder Memory Reuse #54

Merged
merged 2 commits into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/core/lib/BasicOrderFulfiller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ import {
OrderFulfilled_offer_body_offset,
OrderFulfilled_offer_head_offset,
OrderFulfilled_offer_length_baseOffset,
OrderFulfilled_post_memory_region_reservedBytes,
OrderFulfilled_selector,
ReceivedItem_amount_offset,
ReceivedItem_size,
Expand Down Expand Up @@ -1054,7 +1055,16 @@ contract BasicOrderFulfiller is OrderValidator {
mstore(ZeroSlot, 0)

// Update the free memory pointer so that event data is persisted.
mstore(FreeMemoryPointerSlot, add(eventDataPtr, dataSize))
mstore(FreeMemoryPointerSlot,
add(
eventDataPtr,
// reserve extra 3 words to be used by `authorizeOrder` and
// `validatateOrder` if pre-post exection hook to the zone is
// required. These 3 memory slots will be used for the extra data/context
// and order hashes of the calldata.
add(dataSize, OrderFulfilled_post_memory_region_reservedBytes)
)
)
}

// Verify the status of the derived order.
Expand Down
41 changes: 19 additions & 22 deletions src/core/lib/ConsiderationEncoder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity ^0.8.24;

import {
authorizeOrder_calldata_baseOffset,
authorizeOrder_head_offset,
authorizeOrder_selector_offset,
authorizeOrder_selector,
Expand Down Expand Up @@ -554,9 +555,11 @@ contract ConsiderationEncoder {

/**
* @dev Takes an order hash and BasicOrderParameters struct (from calldata)
* and encodes it as `authorizeOrder` calldata. Note that data is
* copied from event data, so this function will need to be modified if
* the layout of that event data changes.
* and encodes it as `authorizeOrder` calldata. Note that memory data is reused
* from `OrderFulfilled` event data, and the rest of the calldata is prefixed and
* postfixed to this memory region. Importantly the memory region before the
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Importantly the memory region before the OrderFulfilled's spent and received items are overwritten

why is it safe to do that? @Saw-mon-and-Natalie

* `OrderFulfilled`'s spent and received items are overwritten to. So this
* function will need to be modified if the layout of that event data changes.
*
* @param orderHash The order hash.
*
Expand All @@ -571,8 +574,19 @@ contract ConsiderationEncoder {
uint256 size,
uint256 memoryLocationForOrderHashes
) {
// Get free memory pointer to write calldata to.
ptr = getFreeMemoryPointer();
unchecked {
// Derive offset to pre `OrderFulfilled`'s spent item event data
// using base offset & total original recipients.
ptr = MemoryPointer.wrap(
Copy link
Collaborator

@MrToph MrToph Mar 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for myself: The original eventDataPtr is @ OrderFulfilled_baseOffset=0x200 + BasicOrder_totalOriginalAdditionalRecipients_cdPtr * 0x20. Here we go to authorizeOrder_calldata_baseOffset=0x80 + BasicOrder_totalOriginalAdditionalRecipients_cdPtr * 0x20. So we're 0x180 in front of it, which is what we want.

authorizeOrder_calldata_baseOffset +
(
CalldataPointer.wrap(
BasicOrder_totalOriginalAdditionalRecipients_cdPtr
).readUint256() <<
OneWordShift
)
);
}

MemoryPointer dst = ptr;

Expand Down Expand Up @@ -619,25 +633,11 @@ contract ConsiderationEncoder {
BasicOrder_additionalRecipients_length_cdPtr
).readUint256();

// Retrieve the length of additional recipients.
uint256 totalOriginalAdditionalRecipientsLength = CalldataPointer.wrap(
BasicOrder_totalOriginalAdditionalRecipients_cdPtr
).readUint256();

// Derive offset to event data using base offset & total recipients.
uint256 offerDataOffset = OrderFulfilled_offer_length_baseOffset
+ (totalOriginalAdditionalRecipientsLength << OneWordShift);

// Derive size of offer and consideration data.
// 2 words (lengths) + 4 (offer data) + 5 (consideration 1) + 5 * ar
uint256 offerAndConsiderationSize = OrderFulfilled_baseDataSize
+ (additionalRecipientsLength * ReceivedItem_size);

// Copy offer and consideration data from event data to calldata.
MemoryPointer.wrap(offerDataOffset).copy(
dstHead.offset(tailOffset), offerAndConsiderationSize
);

// Increment tail offset, now used to populate extraData array.
tailOffset += offerAndConsiderationSize;
}
Expand Down Expand Up @@ -669,9 +669,6 @@ contract ConsiderationEncoder {

// Final size: selector, ZoneParameters pointer, orderHashes & tail.
size = ZoneParameters_basicOrderFixedElements_length + tailOffset;

// Update the free memory pointer.
setFreeMemoryPointer(dst.offset(size + OneWord));
}
}

Expand Down
34 changes: 34 additions & 0 deletions src/types/lib/ConsiderationConstants.sol
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,40 @@ uint256 constant OrderFulfilled_offer_body_offset = 0x80;
uint256 constant OrderFulfilled_consideration_head_offset = 0x60;
uint256 constant OrderFulfilled_consideration_body_offset = 0x120;

/*
* 3 memory slots/words for `authorizeOrder` and `validateOrder` calldata
MrToph marked this conversation as resolved.
Show resolved Hide resolved
* to be used for the extra data/context data and order hashes
*/
uint256 constant OrderFulfilled_post_memory_region_reservedBytes = 0x60;

/*
* OrderFulfilled_offer_length_baseOffset - 12 * 0x20
* we back up 12 words from where the `OrderFulfilled`'s data
* for spent items start to be rewritten for `authorizeOrder`
* and `validateOrder`. Let the reference pointer be `ptr`
* pointing to the `OrderFulfilled`'s spent item array's length memory
* position then we would have:
*
* ptr - 0x0180 : padded calldata selector
* ptr - 0x0160 : ZoneParameter's struct head (0x20)
* ptr - 0x0140 : order hash
* ptr - 0x0120 : fulfiller / msg.sender
* ptr - 0x0100 : offerer
* ptr - 0x00e0 : spent items' head
* ptr - 0x00c0 : received items' head
* ptr - 0x00a0 : extra data / context head
* ptr - 0x0080 : order hashes head
* ptr - 0x0060 : start time
* ptr - 0x0040 : end time
* ptr - 0x0020 : zone hash
* ptr - 0x0000: offer.length (1)
* ...
*
* Note that the padded calldata selector will be at minimum at the
* 0x80 memory slot if no total original additional recipients were provided
MrToph marked this conversation as resolved.
Show resolved Hide resolved
*/
uint256 constant authorizeOrder_calldata_baseOffset = 0x80; // 0x200 - 0x180
MrToph marked this conversation as resolved.
Show resolved Hide resolved

// BasicOrderParameters
uint256 constant BasicOrder_parameters_cdPtr = 0x04;
uint256 constant BasicOrder_considerationToken_cdPtr = 0x24;
Expand Down
Loading