chore: reduce precision of MAX_FEE_ASSET_PRICE_MODIFIER#56
Merged
Conversation
just-mitch
approved these changes
Mar 20, 2025
LHerskind
added a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Mar 24, 2025
Fixes #12870. The changes are fairly simple. Define boundaries for the values that we are going to store as part of the fee header, then use storage that can fit those boundaries but not much bigger. Currently everything was `uint256`. We create a `CompressedFeeHeader` which is the value that we are going to put into storage, and we will define it as follows: ```solidity struct CompressedFeeHeader { uint64 congestionCost; uint64 provingCost; uint48 feeAssetPriceNumerator; uint48 excessMana; uint32 manaUsed; } ``` Following the standard of biggest type first. The `congestionCost` and `provingCost` are both costs per unit of mana, and even if it grows very large, `uint64` should be sufficient. If you need to pay multiple who coins per unit of compute something is wrong. For the `feeAssetPriceNumerator` we update its precision to "only" use 1e6 for 1% diff. With that, we can increase by `1e6` at most per block, so starting 0 we should be able to handle a 1/4 BILLION blocks in a row all increasing to the max - should cover it. This update in precision required the model in engineering designs to also be updated to provide values in the proper size. Therefore the diff looks much bigger as they were updated. The change in engineering designs is in AztecProtocol/engineering-designs#56. For `excessMana` we can at most increase by `manaUsed` each round, and with `uint48` we have a few trillion at hand. At that point, the congestion fee should already be so nasty if even computable that there are separate issues. Lastly the amount of mana used in the specific block is expected to fit in uint32 for a long time. I am happy to plesently surprised if possible for gigablocks, but I don't believe it. 4 billion gas could fit. If running out of space or some vars need to be bigger we can play around with the precision of some measures (such at the costs at the top). --- Comment on gas. If you look at the `gas-report` it won't change drastically, the main reason is that a lot of the tests are using zero values, e.g., updating from zero to zero, which is not nearly as costly as going from nothing to something. A separate funnel for it will short bigger savings, but I expect this to be visible from some of the work on #12614.
DanielKotov
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Mar 27, 2025
Fixes #12870. The changes are fairly simple. Define boundaries for the values that we are going to store as part of the fee header, then use storage that can fit those boundaries but not much bigger. Currently everything was `uint256`. We create a `CompressedFeeHeader` which is the value that we are going to put into storage, and we will define it as follows: ```solidity struct CompressedFeeHeader { uint64 congestionCost; uint64 provingCost; uint48 feeAssetPriceNumerator; uint48 excessMana; uint32 manaUsed; } ``` Following the standard of biggest type first. The `congestionCost` and `provingCost` are both costs per unit of mana, and even if it grows very large, `uint64` should be sufficient. If you need to pay multiple who coins per unit of compute something is wrong. For the `feeAssetPriceNumerator` we update its precision to "only" use 1e6 for 1% diff. With that, we can increase by `1e6` at most per block, so starting 0 we should be able to handle a 1/4 BILLION blocks in a row all increasing to the max - should cover it. This update in precision required the model in engineering designs to also be updated to provide values in the proper size. Therefore the diff looks much bigger as they were updated. The change in engineering designs is in AztecProtocol/engineering-designs#56. For `excessMana` we can at most increase by `manaUsed` each round, and with `uint48` we have a few trillion at hand. At that point, the congestion fee should already be so nasty if even computable that there are separate issues. Lastly the amount of mana used in the specific block is expected to fit in uint32 for a long time. I am happy to plesently surprised if possible for gigablocks, but I don't believe it. 4 billion gas could fit. If running out of space or some vars need to be bigger we can play around with the precision of some measures (such at the costs at the top). --- Comment on gas. If you look at the `gas-report` it won't change drastically, the main reason is that a lot of the tests are using zero values, e.g., updating from zero to zero, which is not nearly as costly as going from nothing to something. A separate funnel for it will short bigger savings, but I expect this to be visible from some of the work on #12614.
DanielKotov
pushed a commit
to AztecProtocol/aztec-packages
that referenced
this pull request
Mar 27, 2025
Fixes #12870. The changes are fairly simple. Define boundaries for the values that we are going to store as part of the fee header, then use storage that can fit those boundaries but not much bigger. Currently everything was `uint256`. We create a `CompressedFeeHeader` which is the value that we are going to put into storage, and we will define it as follows: ```solidity struct CompressedFeeHeader { uint64 congestionCost; uint64 provingCost; uint48 feeAssetPriceNumerator; uint48 excessMana; uint32 manaUsed; } ``` Following the standard of biggest type first. The `congestionCost` and `provingCost` are both costs per unit of mana, and even if it grows very large, `uint64` should be sufficient. If you need to pay multiple who coins per unit of compute something is wrong. For the `feeAssetPriceNumerator` we update its precision to "only" use 1e6 for 1% diff. With that, we can increase by `1e6` at most per block, so starting 0 we should be able to handle a 1/4 BILLION blocks in a row all increasing to the max - should cover it. This update in precision required the model in engineering designs to also be updated to provide values in the proper size. Therefore the diff looks much bigger as they were updated. The change in engineering designs is in AztecProtocol/engineering-designs#56. For `excessMana` we can at most increase by `manaUsed` each round, and with `uint48` we have a few trillion at hand. At that point, the congestion fee should already be so nasty if even computable that there are separate issues. Lastly the amount of mana used in the specific block is expected to fit in uint32 for a long time. I am happy to plesently surprised if possible for gigablocks, but I don't believe it. 4 billion gas could fit. If running out of space or some vars need to be bigger we can play around with the precision of some measures (such at the costs at the top). --- Comment on gas. If you look at the `gas-report` it won't change drastically, the main reason is that a lot of the tests are using zero values, e.g., updating from zero to zero, which is not nearly as costly as going from nothing to something. A separate funnel for it will short bigger savings, but I expect this to be visible from some of the work on #12614.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
As mentioned in AztecProtocol/aztec-packages#12871 we wish to reduce the precision of the fee asset price modifier such that we can hold related values in smaller types and reduce storage overhead.