feat: tracking undelivered request Ids #11
Conversation
kupermind
commented
Oct 23, 2023
kupermind
commented
- Tracking of undelivered request Ids.
| /// @title AgentMech - Smart contract for extending ERC721Mech | ||
| /// @dev A Mech that is operated by the holder of an ERC721 non-fungible token. | ||
| contract AgentMech is ERC721Mech { | ||
| event Perform(address indexed sender, bytes32 taskHash); |
There was a problem hiding this comment.
This event is not used.
| // Map of request Ids | ||
| mapping (uint256 => uint256[2]) public mapRequestIds; |
There was a problem hiding this comment.
A two-way circular map stores all the undelivered requests and correctly removes elements when requests are delivered.
contracts/AgentMech.sol
Outdated
|
|
||
| /// @dev Gets the set of undelivered request Ids. | ||
| /// @return requestIds Set of undelivered request Ids. | ||
| function getUndeliveredRequestIds() external view returns (uint256[] memory requestIds) { |
There was a problem hiding this comment.
Do we need to worry about the number of undelivered requests being too large. Although unlikely, I think it may happen. Could that result in a big request to the rpc which would get dropped constantly?
I am thinking maybe it makes sense to either have this method take a maxNumReqs, or have another method that supports that, just to avoid this case.
There was a problem hiding this comment.
Good point. How about we slightly alter the method to allow to (optionally) set a batch size. Then you can get constant sized returns
There was a problem hiding this comment.
@DavidMinarsch that should do
There was a problem hiding this comment.
I've accounted for both size and offset, please take a look.
| // If size is zero, return all the requests | ||
| if (size == 0) { | ||
| size = numRequests; | ||
| } | ||
|
|
||
| // Check for the size + offset overflow | ||
| if (size + offset > numRequests) { | ||
| revert Overflow(size + offset, numRequests); | ||
| } |
There was a problem hiding this comment.
We need both size and offset to access any number of requests.
| for (uint256 i = 0; i < offset; ++i) { | ||
| // Next request Id of the current element based on the current request Id | ||
| curRequestId = mapRequestIds[curRequestId][1]; | ||
| } |
There was a problem hiding this comment.
Skip the initial offset number of requests.
