Price delayed publications #97
Conversation
…ction of the ProverManager
- Fixes a bug we had where the proposer would always have to send value with their tx - Makes the process more gas efficient, since the inbox does not need to query the fees before
src/protocol/IProverManager.sol
Outdated
| /// @param period The period that the prover is bidding to prove | ||
| /// @param fee The fee that the prover is willing to charge for proving each publication | ||
| /// @param stake The stake that the prover is going to put as stake for the period | ||
| event ProverOffer(address indexed prover, uint256 period, uint256 fee, uint256 stake); |
There was a problem hiding this comment.
nit: This is true for other events, we have period to mean periodID, since we have a Period struct, we should rename to periodId
| /// @notice The percentage (in bps) of the fee that is charged for delayed publications | ||
| /// @dev It is recommended to set this to >10,000 bps since delayed publications should usually be charged at a | ||
| /// higher rate | ||
| uint16 public immutable delayedFeePercentage; |
There was a problem hiding this comment.
This is probably very rare but is uint16 enough? given the percentage is given in bps. (we could only charge up to ~6.5x the normal fee)
There was a problem hiding this comment.
Correct. I have a feeling that's more than enough, but we could make it an uint24 and that would allow ~167.77x. For now it wouldn't do any impact on storage since we are only using 25 bytes, so we still have 7 bytes available in that slot. But when the protocol evolves those bytes might be useful for some other purposes.
Co-authored-by: Leo <leonard.paturel@openzeppelin.com>
Changes to gas cost
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
We still need to use this in the actual code
| /// @inheritdoc IProposerFees | ||
| /// @dev This function advances to the next period if the current period has ended. | ||
| function payPublicationFee(address proposer, bool isDelayed) external { | ||
| require(msg.sender == inbox, "Only the Inbox contract can call this function"); |
There was a problem hiding this comment.
There are several examples of error strings rather than custom errors (as required by the style guide). It's probably worth creating them as we go.
We now use it consistently every time. The intuition is that the overflow is a low-level computer science concept but the rationale about whether it can overflow is a statement about likely business-level configurations. We can optimise it later if it turns out to be prohibitive
Fixes #68
This PR is an attempt to give the flexibility to price delayed publications differently, while still keeping them as part of the fees(more details can be found on the issue).
Pros of this approach:
Cons:
For now it:
Uses more storage since we have a new variable for thedelayedFeePercentage. This shouldn't be an issue once we pack the storage variablesAdds another config parameter(which will be abstracted with an internal function soon)These both are fixed in #100
This PR also:
payPublicationFeefunctionpayPublicationFee. Now we rely on the proposer to deposit before and the call this function. This saves some gas since we don't need to query the fees beforehand