Stylus Cache Manager #36
Conversation
| @@ -47,7 +47,8 @@ | |||
| "@offchainlabs/upgrade-executor": "1.1.0-beta.0", | |||
| "@openzeppelin/contracts": "4.5.0", | |||
| "@openzeppelin/contracts-upgradeable": "4.5.2", | |||
| "patch-package": "^6.4.7" | |||
| "patch-package": "^6.4.7", | |||
| "solady": "^0.0.182" | |||
There was a problem hiding this comment.
prefer to pin contract dependency instead of have it floating
or if we are only using 1 file we can copy it to this repo
| event InsertBid(uint256 bid, bytes32 indexed codehash, uint64 size); | ||
| event DeleteBid(uint256 bid, bytes32 indexed codehash, uint64 size); |
There was a problem hiding this comment.
would be nice to have indexed fields first
| /// Evicts entries until enough space exists in the cache, reverting if payment is insufficient. | ||
| /// Returns the new amount of space available on success. | ||
| /// Note: will only make up to 5Mb of space. Call repeatedly for more. | ||
| function makeSpace(uint64 size) external payable returns (uint64 space) { |
There was a problem hiding this comment.
this is callable when paused, not sure if its expected
There was a problem hiding this comment.
This wasn't necessary for correctness (the point of pausing is to eventually evict everything), but we may as well just let the owner do everything in this case
| index = uint64(entries.length); | ||
|
|
||
| uint256 min; | ||
| while (queueSize + size > cacheSize) { |
There was a problem hiding this comment.
cacheSize can be cached outside the loop to save some SLOAD
| struct Entry { | ||
| bytes32 code; | ||
| uint64 size; | ||
| } |
There was a problem hiding this comment.
we don't need this struct and can save 1 storage slot if we remove size here, and instead call _asmSize(code) when we need the size when removing a bid
There was a problem hiding this comment.
This is a very reasonable suggestion, but it turns out _asmSize does an SLOAD in ArbOS so it's a bit cheaper to keep the struct :)
|
|
||
| /// Creates a packed bid item | ||
| function _packBid(uint256 bid, uint64 index) internal pure returns (uint256) { | ||
| return (bid << 64) | uint256(index); |
There was a problem hiding this comment.
this is unlikely to be reachable unless misconfigured decay, but we should check for bid < type(uint192).max before shifting or might as well use uint192 for all bid values
There was a problem hiding this comment.
Updated the math to use 192-bit numbers, but this shouldn't be reachable since decay is 64-bit
| function setCacheSize(uint64 newSize) external onlyOwner { | ||
| cacheSize = newSize; | ||
| } | ||
|
|
||
| /// Sets the intended decay factor. Does not modify existing bids. | ||
| function setDecayRate(uint64 newDecay) external onlyOwner { | ||
| decay = newDecay; | ||
| } |
| if (size > MAX_MAKE_SPACE) { | ||
| size = MAX_MAKE_SPACE; | ||
| } |
There was a problem hiding this comment.
would prefer revert instead of capping if it requested to make more space than allowed
This PR introduces new precompiles to support program & trie slot caching