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: add total mana used to header #9868

Merged
merged 3 commits into from
Nov 27, 2024
Merged

Conversation

LHerskind
Copy link
Contributor

@LHerskind LHerskind commented Nov 9, 2024

Fixes #9716.

  • Introduces total_mana_used value into the block header (sum of mana used in txs of block)
  • Updates an issue in the integration_l1_publisher as it did not "reset" the list of transactions between blocks, e.g., if block 1 = [a,b,c,d] then block 2 which should have been [e,f,g,h] became [a,b,c,d,e,f,g,h].
  • Updated an issue in the updateInlineTestData function, as it required at least one space before a newline, which is not what nargo fmt produces.
  • Add estimatedGasPad to SendMethodOptions to allow specifying padding more easily.
  • Updated derivation.test.ts because it had a bunch of invalid paths for noir code or and some missing tests
  • Setting the gas used to 0, in the processed tx mocks!, see bug: Orchestrator tests explodes when gas and fees are non-zero #9900
  • Increases a timeout because slow ci made it fail sometimes.

Copy link
Contributor Author

LHerskind commented Nov 9, 2024

@LHerskind LHerskind marked this pull request as ready for review November 9, 2024 23:32
@LHerskind LHerskind added e2e-all CI: Enables this CI job. network-all Run this CI job. labels Nov 9, 2024
Copy link
Contributor

github-actions bot commented Nov 10, 2024

Changes to circuit sizes

Generated at commit: dece29817233bd7d1ba639c41482d2953a8edc2a, compared to commit: 7b2e343a61eb9c74f365758530deca87b40891d0

🧾 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%

@AztecBot
Copy link
Collaborator

AztecBot commented Nov 10, 2024

Docs Preview

Hey there! 👋 You can check your preview at https://6746fc93a2f1c0feee144a78--aztec-docs-dev.netlify.app

const block = await t.pxe.getBlock(withEstimate.blockNumber!);

// Note that there is NO mana spent on the teardown in any, even though the user will pay for it in
// the transaction that does not have estimateGas on.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not following this comment.

@@ -66,7 +66,7 @@ export function updateInlineTestData(targetFileFromRepoRoot: string, itemName: s
const logger = createConsoleLogger('aztec:testing:test_data');
const targetFile = getPathToFile(targetFileFromRepoRoot);
const contents = readFileSync(targetFile, 'utf8').toString();
const regex = new RegExp(`let ${itemName} = [\\s\\S]*?;`, 'g');
const regex = new RegExp(`let ${itemName} =[\\s\\S]*?;`, 'g');
Copy link
Contributor

Choose a reason for hiding this comment

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

😬. Not to your fix, but generally.

Copy link
Contributor

@just-mitch just-mitch left a comment

Choose a reason for hiding this comment

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

Minor comment. Looks good to me.

@@ -203,6 +203,14 @@ fn compute_address_from_partial_and_pub_keys() {
assert(address.to_field() == expected_computed_address_from_partial_and_pubkeys);
}

#[test]
fn compute_preaddress_from_partial_and_pub_keys() {
Copy link
Member

Choose a reason for hiding this comment

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

this test confuses me, there is neither a partial or public key in it. Its 3 constant values hashed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ye now sure what it is. But it was a ts test that expected a matching noir, might be that it should have been removed, but seems like the inner have is used in both noir and ts but just had its tet deleted in the past so just added it to silence the errors when running snapshot.

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.

remove the test? its not testing what it says it is

Base automatically changed from lh/9722 to master November 26, 2024 21:44
@LHerskind LHerskind changed the base branch from master to lh/public-payments-fix November 26, 2024 22:59
Base automatically changed from lh/public-payments-fix to master November 26, 2024 23:36
@LHerskind LHerskind marked this pull request as ready for review November 27, 2024 00:07

return header;
}

function toFields(Header memory _header) internal pure returns (bytes32[] memory) {
bytes32[] memory fields = new bytes32[](24);
bytes32[] memory fields = new bytes32[](25);
Copy link
Member

Choose a reason for hiding this comment

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

This can read from the constants no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but they are also deleted in the blob pr, so did not really bother too much with creating a constant and such here.

logGasEstimate(estimatedGas);

const [withEstimate, withoutEstimate] = await sendTransfers(paymentMethod);
const actualFee = withEstimate.transactionFee!;

if (withEstimate.blockNumber !== withoutEstimate.blockNumber) {
Copy link
Member

Choose a reason for hiding this comment

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

Could this edge case be avoided by setting the node config minTxPerBlock to 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

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.

lgtm, minor comments

@LHerskind
Copy link
Contributor Author

The artifact hash is likely changed because of the header update, so we gotta update it as well.

@LHerskind LHerskind removed the network-all Run this CI job. label Nov 27, 2024
@LHerskind LHerskind merged commit 2478d19 into master Nov 27, 2024
111 checks passed
@LHerskind LHerskind deleted the lh/9716-total-mana-header branch November 27, 2024 12:07
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
e2e-all CI: Enables this CI job.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: add total_mana_used to L2 block header
4 participants