Skip to content
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

feat: burn congestion fee #10231

Merged
merged 2 commits into from
Nov 27, 2024
Merged

feat: burn congestion fee #10231

merged 2 commits into from
Nov 27, 2024

Conversation

LHerskind
Copy link
Contributor

@LHerskind LHerskind commented Nov 26, 2024

Fixes #10063 and #10209

Uses a bit more storage than we would like, but it is to be taken care of as the separate storage optimization issue.

Uses structs for a few of the functions instead of the usual positioned args to work around a bit of stack too deep issues.

Moves the sample lib to linked library to reduce the contract size of the rollup contract.

Updates slightly in the way linked libraries are done, as there was an issue when multiple linked libraries needed.

Copy link
Contributor Author

LHerskind commented Nov 26, 2024

@LHerskind LHerskind changed the title feat: burn baby burn feat: burn congestion fee Nov 26, 2024
@LHerskind LHerskind force-pushed the lh/9716-total-mana-header branch from 42e2ebb to 64924a4 Compare November 27, 2024 00:07
require(
interimValues.startEpoch == interimValues.epochToProve,
Errors.Rollup__InvalidEpoch(interimValues.startEpoch, interimValues.epochToProve)
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interimValues.epochToProve must equal proofClaim.epochToProve, i may be missing where this assertion is

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok the assertion is at the bottom, they dont get their bond back if they do not prove the epoch in the claim

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have that assertion, it is an if for freeing bond. Will move it up to make it easier to follow 🫡

@@ -81,6 +90,10 @@ contract Rollup is EIP712("Aztec Rollup", "1"), Leonidas, IRollup, ITestRollup {
// for justification of CLAIM_DURATION_IN_L2_SLOTS.
uint256 public constant PROOF_COMMITMENT_MIN_BOND_AMOUNT_IN_TST = 1000;

// A Cuauhxicalli [kʷaːʍʃiˈkalːi] ("eagle gourd bowl") is a ceremonial Aztec vessel or altar used to hold offerings,
// such as sacrificial hearts, during rituals performed within temples.
address public constant CUAUHXICALLI = address(bytes20("CUAUHXICALLI"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my minds tellin me noooooo, but my body, my body'ss tellin me yeessss

];

aggregationObject = Buffer.from(hexToBytes(decodedArgs.aggregationObject));
proverId = Fr.fromString(decodedArgs.args[6]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit decodedArgs.args doesnt feel right as a name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly for consistency with the other that is decoding.

@@ -726,9 +725,55 @@ contract RollupTest is DecoderBase, TimeFns {
assertEq(rollup.getProvenBlockNumber(), 0 + toProve, "Invalid proven block number");
}

function testRevertBlocksAcrossEpochs() public setUpFor("mixed_block_1") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets update the name of this test - it is not clear that this is submitting proofs for blocks across epochs rather than proposing two blocks across epochs

testRevertSubmittingProofForBlocksAcrossEpochs

@@ -104,6 +126,13 @@ contract FeeRollupTest is FeeModelTestPoints, DecoderBase {
aztecEpochProofClaimWindowInL2Slots: 16
})
);
fakeCanonical.setCanonicalRollup(address(rollup));

vm.label(moneyMaker, "MONEY MAKER");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feeRecipient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feeRecipient is a keyword, and it is not just getting fees but also the rewards. Were thinking coinbase first but here it is also the "prover" so just went with money maker as I was tired.

@@ -181,9 +212,8 @@ contract FeeRollupTest is FeeModelTestPoints, DecoderBase {
}

function test_BigBrainTime() public {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may aswell continue with the naming policing while im here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😭


uint256 burned = asset.balanceOf(rollup.CUAUHXICALLI()) - cuauhxicalliBalanceBefore;
assertEq(
asset.balanceOf(moneyMaker) - moneyMakerBalanceBefore - 50e18 * epochSize + burned,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constant for 50e18


uint256 feeSum = 0;
uint256 burnSum = 0;
bytes32[] memory fees = new bytes32[](Constants.AZTEC_MAX_EPOCH_DURATION * 2);
Copy link
Member

@Maddiaa0 Maddiaa0 Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remark: we should probbaly make these fee arrays two arrays, one for receipient and one for the amount, where these are recipient fee pairs is a bit of a mental overhead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also they don't really need to have the size of the epoch, but only the ones that are actually used etc 🤷

Copy link
Member

@Maddiaa0 Maddiaa0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic looks good, as discussed we need to start cleaning up, im having to swap between alot of places to read alot of this stuff.

all comments are preferences

@LHerskind LHerskind force-pushed the lh/9716-total-mana-header branch 2 times, most recently from c5fdb40 to 3cc2779 Compare November 27, 2024 09:55
@LHerskind LHerskind linked an issue Nov 27, 2024 that may be closed by this pull request
Base automatically changed from lh/9716-total-mana-header to master November 27, 2024 12:07
@LHerskind LHerskind marked this pull request as ready for review November 27, 2024 12:09
Copy link
Contributor

Changes to circuit sizes

Generated at commit: a8efa4077a52b0865605da3c598eeb0b2be7df53, compared to commit: 0c7c4c9bb0c01067abe57ccd06962d71c7279aa0

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_init +2 ❌ +0.01% +3 ❌ +0.01%
private_kernel_tail +2 ❌ +0.05% +1 ❌ +0.01%
private_kernel_inner +3 ❌ +0.01% +3 ❌ +0.01%
private_kernel_reset_4_4_4_4_4_4_4_4_1 +2 ❌ +0.01% +3 ❌ +0.00%
private_kernel_tail_to_public +2 ❌ +0.01% +1 ❌ +0.00%
rollup_base_private +2 ❌ +0.00% +83 ❌ +0.00%
rollup_merge +1 ❌ +0.03% +13 ❌ +0.00%
rollup_block_root +1 ❌ +0.02% +13 ❌ +0.00%
private_kernel_reset +2 ❌ +0.00% +2 ❌ +0.00%
rollup_base_public +2 ❌ +0.00% +8 ❌ +0.00%
private_kernel_empty +1 ❌ +0.16% +1 ❌ +0.00%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
private_kernel_init 21,409 (+2) +0.01% 34,506 (+3) +0.01%
private_kernel_tail 4,133 (+2) +0.05% 12,515 (+1) +0.01%
private_kernel_inner 37,552 (+3) +0.01% 56,882 (+3) +0.01%
private_kernel_reset_4_4_4_4_4_4_4_4_1 32,387 (+2) +0.01% 86,494 (+3) +0.00%
private_kernel_tail_to_public 17,386 (+2) +0.01% 29,505 (+1) +0.00%
rollup_base_private 197,675 (+2) +0.00% 2,658,662 (+83) +0.00%
rollup_merge 3,420 (+1) +0.03% 1,827,059 (+13) +0.00%
rollup_block_root 4,490 (+1) +0.02% 2,739,665 (+13) +0.00%
private_kernel_reset 84,029 (+2) +0.00% 618,039 (+2) +0.00%
rollup_base_public 447,127 (+2) +0.00% 3,997,451 (+8) +0.00%
private_kernel_empty 613 (+1) +0.16% 901,640 (+1) +0.00%

@LHerskind LHerskind merged commit 20a33f2 into master Nov 27, 2024
108 of 109 checks passed
@LHerskind LHerskind deleted the lh/10063 branch November 27, 2024 12:51
critesjosh pushed a commit that referenced this pull request Nov 27, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.65.1</summary>

##
[0.65.1](aztec-package-v0.65.0...aztec-package-v0.65.1)
(2024-11-27)


### Miscellaneous

* Delete old serialization methods
([#9951](#9951))
([10d3f6f](10d3f6f))
</details>

<details><summary>barretenberg.js: 0.65.1</summary>

##
[0.65.1](barretenberg.js-v0.65.0...barretenberg.js-v0.65.1)
(2024-11-27)


### Features

* Speed up transaction execution
([#10172](#10172))
([da265b6](da265b6))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](#10186))
([b773c14](b773c14))
</details>

<details><summary>aztec-packages: 0.65.1</summary>

##
[0.65.1](aztec-packages-v0.65.0...aztec-packages-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](#9868))
([2478d19](2478d19))
* Assert metrics in network tests
([#10215](#10215))
([9380c0f](9380c0f))
* Avm inserts nullifiers from private
([#10129](#10129))
([3fc0c7c](3fc0c7c))
* Burn congestion fee
([#10231](#10231))
([20a33f2](20a33f2))
* Configure world state block history
([#10216](#10216))
([01eb392](01eb392))
* Integrate fee into rollup
([#10176](#10176))
([12744d6](12744d6))
* Speed up transaction execution
([#10172](#10172))
([da265b6](da265b6))
* Using current gas prices in cli-wallet
([#10105](#10105))
([15ffeea](15ffeea))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](#10186))
([b773c14](b773c14))
* **avm:** Execution test ordering
([#10226](#10226))
([49b4a6c](49b4a6c))
* Deploy preview master
([#10227](#10227))
([321a175](321a175))
* Use current base fee for public fee payment
([#10230](#10230))
([f081d80](f081d80))


### Miscellaneous

* Add traces and histograms to avm simulator
([#10233](#10233))
([e83726d](e83726d)),
closes
[#10146](#10146)
* Avm-proving and avm-integration tests do not require simulator to
export function with jest mocks
([#10228](#10228))
([f28fcdb](f28fcdb))
* **avm:** Handle parsing error
([#10203](#10203))
([3c623fc](3c623fc)),
closes
[#9770](#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](#10238))
([0c7c4c9](0c7c4c9))
* Bump timeout for after-hook for data store test again
([#10240](#10240))
([52047f0](52047f0))
* CIVC VK
([#10223](#10223))
([089c34c](089c34c))
* Declare global types
([#10206](#10206))
([7b2e343](7b2e343))
* Delete old serialization methods
([#9951](#9951))
([10d3f6f](10d3f6f))
* Fix migration notes
([#10252](#10252))
([05bdcd5](05bdcd5))
* Pull out some sync changes
([#10245](#10245))
([1bfc15e](1bfc15e))
* Remove docs from sync
([#10241](#10241))
([eeea0aa](eeea0aa))
* Replace relative paths to noir-protocol-circuits
([e7690ca](e7690ca))
</details>

<details><summary>barretenberg: 0.65.1</summary>

##
[0.65.1](barretenberg-v0.65.0...barretenberg-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](#9868))
([2478d19](2478d19))
* Configure world state block history
([#10216](#10216))
([01eb392](01eb392))
* Speed up transaction execution
([#10172](#10172))
([da265b6](da265b6))


### Bug Fixes

* **avm:** Execution test ordering
([#10226](#10226))
([49b4a6c](49b4a6c))


### Miscellaneous

* **avm:** Handle parsing error
([#10203](#10203))
([3c623fc](3c623fc)),
closes
[#9770](#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](#10238))
([0c7c4c9](0c7c4c9))
* CIVC VK
([#10223](#10223))
([089c34c](089c34c))
* Pull out some sync changes
([#10245](#10245))
([1bfc15e](1bfc15e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
AztecBot added a commit to AztecProtocol/barretenberg that referenced this pull request Nov 28, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>aztec-package: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@aztec-package-v0.65.0...aztec-package-v0.65.1)
(2024-11-27)


### Miscellaneous

* Delete old serialization methods
([#9951](AztecProtocol/aztec-packages#9951))
([10d3f6f](AztecProtocol/aztec-packages@10d3f6f))
</details>

<details><summary>barretenberg.js: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@barretenberg.js-v0.65.0...barretenberg.js-v0.65.1)
(2024-11-27)


### Features

* Speed up transaction execution
([#10172](AztecProtocol/aztec-packages#10172))
([da265b6](AztecProtocol/aztec-packages@da265b6))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](AztecProtocol/aztec-packages#10186))
([b773c14](AztecProtocol/aztec-packages@b773c14))
</details>

<details><summary>aztec-packages: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@aztec-packages-v0.65.0...aztec-packages-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](AztecProtocol/aztec-packages#9868))
([2478d19](AztecProtocol/aztec-packages@2478d19))
* Assert metrics in network tests
([#10215](AztecProtocol/aztec-packages#10215))
([9380c0f](AztecProtocol/aztec-packages@9380c0f))
* Avm inserts nullifiers from private
([#10129](AztecProtocol/aztec-packages#10129))
([3fc0c7c](AztecProtocol/aztec-packages@3fc0c7c))
* Burn congestion fee
([#10231](AztecProtocol/aztec-packages#10231))
([20a33f2](AztecProtocol/aztec-packages@20a33f2))
* Configure world state block history
([#10216](AztecProtocol/aztec-packages#10216))
([01eb392](AztecProtocol/aztec-packages@01eb392))
* Integrate fee into rollup
([#10176](AztecProtocol/aztec-packages#10176))
([12744d6](AztecProtocol/aztec-packages@12744d6))
* Speed up transaction execution
([#10172](AztecProtocol/aztec-packages#10172))
([da265b6](AztecProtocol/aztec-packages@da265b6))
* Using current gas prices in cli-wallet
([#10105](AztecProtocol/aztec-packages#10105))
([15ffeea](AztecProtocol/aztec-packages@15ffeea))


### Bug Fixes

* Add pako as a dependency in bb.js
([#10186](AztecProtocol/aztec-packages#10186))
([b773c14](AztecProtocol/aztec-packages@b773c14))
* **avm:** Execution test ordering
([#10226](AztecProtocol/aztec-packages#10226))
([49b4a6c](AztecProtocol/aztec-packages@49b4a6c))
* Deploy preview master
([#10227](AztecProtocol/aztec-packages#10227))
([321a175](AztecProtocol/aztec-packages@321a175))
* Use current base fee for public fee payment
([#10230](AztecProtocol/aztec-packages#10230))
([f081d80](AztecProtocol/aztec-packages@f081d80))


### Miscellaneous

* Add traces and histograms to avm simulator
([#10233](AztecProtocol/aztec-packages#10233))
([e83726d](AztecProtocol/aztec-packages@e83726d)),
closes
[#10146](AztecProtocol/aztec-packages#10146)
* Avm-proving and avm-integration tests do not require simulator to
export function with jest mocks
([#10228](AztecProtocol/aztec-packages#10228))
([f28fcdb](AztecProtocol/aztec-packages@f28fcdb))
* **avm:** Handle parsing error
([#10203](AztecProtocol/aztec-packages#10203))
([3c623fc](AztecProtocol/aztec-packages@3c623fc)),
closes
[#9770](AztecProtocol/aztec-packages#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](AztecProtocol/aztec-packages#10238))
([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9))
* Bump timeout for after-hook for data store test again
([#10240](AztecProtocol/aztec-packages#10240))
([52047f0](AztecProtocol/aztec-packages@52047f0))
* CIVC VK
([#10223](AztecProtocol/aztec-packages#10223))
([089c34c](AztecProtocol/aztec-packages@089c34c))
* Declare global types
([#10206](AztecProtocol/aztec-packages#10206))
([7b2e343](AztecProtocol/aztec-packages@7b2e343))
* Delete old serialization methods
([#9951](AztecProtocol/aztec-packages#9951))
([10d3f6f](AztecProtocol/aztec-packages@10d3f6f))
* Fix migration notes
([#10252](AztecProtocol/aztec-packages#10252))
([05bdcd5](AztecProtocol/aztec-packages@05bdcd5))
* Pull out some sync changes
([#10245](AztecProtocol/aztec-packages#10245))
([1bfc15e](AztecProtocol/aztec-packages@1bfc15e))
* Remove docs from sync
([#10241](AztecProtocol/aztec-packages#10241))
([eeea0aa](AztecProtocol/aztec-packages@eeea0aa))
* Replace relative paths to noir-protocol-circuits
([e7690ca](AztecProtocol/aztec-packages@e7690ca))
</details>

<details><summary>barretenberg: 0.65.1</summary>

##
[0.65.1](AztecProtocol/aztec-packages@barretenberg-v0.65.0...barretenberg-v0.65.1)
(2024-11-27)


### Features

* Add total mana used to header
([#9868](AztecProtocol/aztec-packages#9868))
([2478d19](AztecProtocol/aztec-packages@2478d19))
* Configure world state block history
([#10216](AztecProtocol/aztec-packages#10216))
([01eb392](AztecProtocol/aztec-packages@01eb392))
* Speed up transaction execution
([#10172](AztecProtocol/aztec-packages#10172))
([da265b6](AztecProtocol/aztec-packages@da265b6))


### Bug Fixes

* **avm:** Execution test ordering
([#10226](AztecProtocol/aztec-packages#10226))
([49b4a6c](AztecProtocol/aztec-packages@49b4a6c))


### Miscellaneous

* **avm:** Handle parsing error
([#10203](AztecProtocol/aztec-packages#10203))
([3c623fc](AztecProtocol/aztec-packages@3c623fc)),
closes
[#9770](AztecProtocol/aztec-packages#9770)
* **avm:** Zero initialization in avm public inputs and execution test
fixes
([#10238](AztecProtocol/aztec-packages#10238))
([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9))
* CIVC VK
([#10223](AztecProtocol/aztec-packages#10223))
([089c34c](AztecProtocol/aztec-packages@089c34c))
* Pull out some sync changes
([#10245](AztecProtocol/aztec-packages#10245))
([1bfc15e](AztecProtocol/aztec-packages@1bfc15e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: update fee payouts
2 participants