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

fix: deploy l2 contracts fails on 48 validator #9860

Merged
merged 7 commits into from
Nov 11, 2024

Conversation

just-mitch
Copy link
Collaborator

This was sometimes erroring out on the first attempt while awaiting the transaction to settle,
but then subsequent calls were attempting to reinitialize.

Added better logging to help clarify.

Also adjust the validator url in the template to be the top level service, to better distribute transactions across the network and not rely so heavily on the boot node for gossiping.

@just-mitch just-mitch linked an issue Nov 8, 2024 that may be closed by this pull request
@just-mitch just-mitch changed the title fix: bug deploy l2 contracts fails on 48 validator fix: deploy l2 contracts fails on 48 validator Nov 8, 2024
@just-mitch just-mitch added the hotfix A PR/issue that needs to be cherrypicked back to the RC label Nov 8, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

Changes to public function bytecode sizes

Generated at commit: 7460fc52119a6f78fe96d6ef7d2c5db52779e208, compared to commit: 6554122bdcd6d3840d03fdf1e7896f3961021e1f

🧾 Summary (100% most significant diffs)

Program Bytecode size in bytes (+/-) %
FPC::public_dispatch +1,034 ❌ +12.37%
FPC::constructor -96 ✅ -3.05%

Full diff report 👇
Program Bytecode size in bytes (+/-) %
FPC::public_dispatch 9,396 (+1,034) +12.37%
FPC::constructor 3,056 (-96) -3.05%

@ludamad ludamad enabled auto-merge (squash) November 9, 2024 06:43
@ludamad ludamad disabled auto-merge November 9, 2024 06:44
await provenTx.send().wait(waitOpts);
log('setupCanonicalL2FeeJuice: Fee juice contract initialized');
} catch (e: any) {
if (e instanceof Error && e.message.includes('(method pxe_simulateTx) (code 400) Assertion failed')) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Noting that this looks almost guaranteed to churn at some point (method name change, format change etc)

Copy link
Collaborator

Choose a reason for hiding this comment

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

correct solution would be maybe 204 error code for this case

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added a todo for now

@ludamad ludamad merged commit 5be770f into master Nov 11, 2024
2 checks passed
@ludamad ludamad deleted the 9822-bug-deploy-l2-contracts-fails-on-48-validator branch November 11, 2024 06:54
Copy link
Contributor

Changes to circuit sizes

Generated at commit: 7460fc52119a6f78fe96d6ef7d2c5db52779e208, compared to commit: 6554122bdcd6d3840d03fdf1e7896f3961021e1f

🧾 Summary (100% most significant diffs)

Program ACIR opcodes (+/-) % Circuit size (+/-) %
rollup_block_root -464 ✅ -10.35% -5,960 ✅ -0.21%
rollup_block_root_empty -32 ✅ -34.04% -32 ✅ -1.11%
rollup_block_merge -6,944 ✅ -57.86% -29,107 ✅ -1.50%
rollup_root -6,944 ✅ -57.94% -29,107 ✅ -1.50%
rollup_base_public -15,774 ✅ -3.36% -221,618 ✅ -5.88%
rollup_base_private -15,774 ✅ -4.74% -221,618 ✅ -6.46%
private_kernel_reset_4_4_4_4_4_4_4_4_1 -667 ✅ -2.06% -9,667 ✅ -11.16%
private_kernel_reset -10,767 ✅ -12.80% -155,407 ✅ -25.14%

Full diff report 👇
Program ACIR opcodes (+/-) % Circuit size (+/-) %
rollup_block_root 4,021 (-464) -10.35% 2,857,288 (-5,960) -0.21%
rollup_block_root_empty 62 (-32) -34.04% 2,859 (-32) -1.11%
rollup_block_merge 5,057 (-6,944) -57.86% 1,911,809 (-29,107) -1.50%
rollup_root 5,041 (-6,944) -57.94% 1,911,795 (-29,107) -1.50%
rollup_base_public 454,278 (-15,774) -3.36% 3,549,032 (-221,618) -5.88%
rollup_base_private 317,095 (-15,774) -4.74% 3,210,904 (-221,618) -6.46%
private_kernel_reset_4_4_4_4_4_4_4_4_1 31,789 (-667) -2.06% 76,935 (-9,667) -11.16%
private_kernel_reset 73,331 (-10,767) -12.80% 462,711 (-155,407) -25.14%

ludamad added a commit that referenced this pull request Nov 11, 2024
This was sometimes erroring out on the first attempt while awaiting the
transaction to settle,
but then subsequent calls were attempting to reinitialize.

Added better logging to help clarify. 

Also adjust the validator url in the template to be the top level
service, to better distribute transactions across the network and not
rely so heavily on the boot node for gossiping.

---------

Co-authored-by: ludamad <[email protected]>
ludamad added a commit that referenced this pull request Nov 11, 2024
TomAFrench added a commit that referenced this pull request Nov 11, 2024
* master: (182 commits)
  feat(avm): mem specific range check (#9828)
  refactor: remove public kernel inner (#9865)
  chore: Revert "chore: Validate RPC inputs" (#9875)
  Revert "fix: deploy l2 contracts fails on 48 validator" (#9871)
  fix: deploy l2 contracts fails on 48 validator (#9860)
  chore: Validate RPC inputs (#9672)
  fix: fixing devcontainers to use the sandbox docker-compose file (#9782)
  fix: Revert changes to ci.yml (#9863)
  chore: Move epoch and slot durations to config (#9861)
  git subrepo push --branch=master noir-projects/aztec-nr
  git_subrepo.sh: Fix parent in .gitrepo file. [skip ci]
  chore: replace relative paths to noir-protocol-circuits
  git subrepo push --branch=master barretenberg
  fix: tree heights that last past 3 days (#9760)
  fix(build): l1-contracts .rebuild_patterns did not cover test files (#9862)
  fix: bench prover test (#9856)
  fix: Fix mac build by calling `count` on durations (#9855)
  feat: zk shplemini (#9830)
  feat: domain separate block proposals and attestations (#9842)
  chore: bump runner cache disk size (#9849)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix A PR/issue that needs to be cherrypicked back to the RC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: deploy l2 contracts fails on 48 validator
2 participants