-
-
Notifications
You must be signed in to change notification settings - Fork 88
Added MultiTokenPeriodEnforcer #81
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
McOso
reviewed
Apr 7, 2025
Contributor
Author
|
Just for history context of this hash+cache idea struct TokenIndex {
bool isSaved;
uint256 index;
}
mapping(bytes32 => TokenIndex) public tokenIndex;
function getTermsInfo(
bytes calldata _terms,
address _token,
bytes32 _delegationHash
)
public
returns (uint256 periodAmount_, uint256 periodDuration_, uint256 startDate_)
{
uint256 termsLength_ = _terms.length;
require(termsLength_ != 0 && termsLength_ % 116 == 0, "MultiTokenPeriodEnforcer:invalid-terms-length");
bytes32 indexKey_ = keccak256(abi.encodePacked(msg.sender, _delegationHash, _token));
TokenIndex storage tokenIndex_ = tokenIndex[indexKey_];
if (tokenIndex_.isSaved) return _getTerms(_terms, tokenIndex_.index * 116);
// Iterate over the byte offset directly in increments of 116 bytes.
uint256 index_;
for (uint256 offset_ = 0; offset_ < termsLength_;) {
// Extract token address from the first 20 bytes.
address token_ = address(bytes20(_terms[offset_:offset_ + 20]));
if (token_ == _token) {
tokenIndex[indexKey_] = TokenIndex({ isSaved: true, index: index_ });
return _getTerms(_terms, offset_);
}
unchecked {
++index_;
offset_ += 116;
}
}
revert("MultiTokenPeriodEnforcer:token-config-not-found");
} |
McOso
previously approved these changes
Apr 10, 2025
0fa8f5d to
49818d4
Compare
McOso
approved these changes
Apr 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Why?
How?
Gas and design
While creating this enforcer an idea for improving gas cost was implemented, this idea consisted on hashing the msg.sender, delegationHash, and token to store the index on the state and avoid looping everytime through the token list.
Gas test comparisons were made to evaluate that idea. We were able to see improvements, but the highest gas savings come from using large token lists. After talking to the team internally, we found that 5 tokens will be the most common list, so because of that we stick to the idea of looping through the tokens. The loop doesn't process all the tokens it only loops to find a token match.
Below are the gas savings for comparison.
The gas is only for the function getTermsInfo, which looks for the match in the token. I compared the current version (No store on state) against the version that uses the hash+cache
Using 100 tokens:
No store on state: 48881 constant
Storing index on state: 87930, first time storing, but then it goes down to gas used: 2072, constant forever, it pays up on the 2nd run.
Using 20 tokens:
No store on state: getTermsInfo gas used: 10801 constant
Storing index on state: 54570, first time storing, but then it goes down to gas used: 2072, constant forever, it pays up on the 7th run.
Using 10 tokens:
No store on state: getTermsInfo gas used: 6041 constant
Storing index on state: 50400 first time storing, but then it goes down to gas used: 2072, constant forever, it pays up on the 13th run.
Using 5 tokens:
No store on state: getTermsInfo gas used: 3661 constant
Storing index on state: 48315, first time storing, but then it goes down to gas used: 2072, constant forever, it pays up on the 30th run.
Using 3 tokens:
No store on state: getTermsInfo gas used: 2709 constant
Storing index on state: 47481 first time storing, but then it goes down to gas used: 2072 constant forever, it pays up on the 72th run.
Using 2 tokens:
No store on state: getTermsInfo gas used: 2233 constant
Storing index on state: 47064 first time storing, but then it goes down to gas used: 2072 constant forever, it pays up on the 281th run.