Gen 1261 update claimer to dynamically compute decay constant from #30
Conversation
This file contains 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
asselstine
changed the base branch from
main
to
gen-1260-fix-bug-in-claimer-prize-count
trmid
requested changes
Apr 2, 2024
src/Claimer.sol
Outdated
|
|
||
| /// @notice Computes the decay constant for the VRGDA. | ||
| /// @dev This is a decay constant that ensures the fee will grow to the target max fee | ||
| /// @param _targetFee The starting fee |
There was a problem hiding this comment.
Suggested change
| /// @param _targetFee The starting fee | |
| /// @param _targetFee The target fee |
src/Claimer.sol
Outdated
| } else { | ||
| return _computeMaxFee(_tier); | ||
| } | ||
| } | ||
|
|
||
| /// @notice Compute the starting fee for prize claims | ||
| /// @return The starting fee for prize claims |
There was a problem hiding this comment.
Suggested change
| /// @return The starting fee for prize claims | |
| /// @return The target fee for prize claims |
src/Claimer.sol
Outdated
| @@ -295,23 +296,23 @@ contract Claimer { | |||
| } | |||
|
|
|||
| /// @notice Computes the fee for the next claim. | |||
| /// @param _minimumFee The minimum fee that should be charged | |||
| /// @param _targetFee The minimum fee that should be charged | |||
There was a problem hiding this comment.
Suggested change
| /// @param _targetFee The minimum fee that should be charged | |
| /// @param _targetFee The target fee that should be charged |
| /// @param _targetFee The starting fee | ||
| /// @return The decay constant | ||
| function _computeDecayConstant(uint256 _targetFee) internal view returns (SD59x18) { | ||
| uint maximumFee = _computeMaxFee(0); |
There was a problem hiding this comment.
Would it help the auction price discovery if we multiplied this by the maxFeePortionOfPrize fraction?
There was a problem hiding this comment.
Ah yes! It definitely should; that would make it more efficient
There was a problem hiding this comment.
Ah wait, it already is in the _computeMaxFee function
src/Claimer.sol
Outdated
| } | ||
|
|
||
| /// @notice Computes the decay constant for the VRGDA. | ||
| /// @dev This is a decay constant that ensures the fee will grow to the target max fee |
There was a problem hiding this comment.
This is a bit confusing. Is it growing to the target fee or the max fee?
src/Claimer.sol
Outdated
| } else { | ||
| return _computeMaxFee(_tier); | ||
| } | ||
| } | ||
|
|
||
| /// @notice Compute the starting fee for prize claims |
There was a problem hiding this comment.
Suggested change
| /// @notice Compute the starting fee for prize claims | |
| /// @notice Compute the target fee for prize claims |
asselstine
force-pushed
the
gen-1260-fix-bug-in-claimer-prize-count
branch
from
f333899
to
37d69cc
Compare
asselstine
force-pushed
the
gen-1261-update-claimer-to-dynamically-compute-decay-constant-from
branch
from
633349d
to
13c18cc
Compare
LCOV of commit
|
asselstine
deleted the
gen-1261-update-claimer-to-dynamically-compute-decay-constant-from
branch
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.
No description provided.