Skip to content

test(consensus): declare HelloStarknet in the first proposal, assert transactions for reverts#3270

Merged
CHr15F0x merged 6 commits intomainfrom
chris/declares-in-dummy-proposals
Mar 13, 2026
Merged

test(consensus): declare HelloStarknet in the first proposal, assert transactions for reverts#3270
CHr15F0x merged 6 commits intomainfrom
chris/declares-in-dummy-proposals

Conversation

@CHr15F0x
Copy link
Member

@CHr15F0x CHr15F0x commented Mar 11, 2026

It turns out that our proposal processing logic did not store successfully declared classes. Which then resulted in deployments failing. This was not covered by the integration tests, which passed happily.


List of changes:

  • the first proposal declares HelloStarknet,
  • syncing from consensus now stores the class declaration,
  • consensus integration tests check if all the transactions are not reverted, which solves the issue of silent failures (ie. formerly transactions that the test setup required to be executed successfully got reverted which was still a valid outcome of execution as such but not the test as such, and we didn't inspect the receipts while only looking at decided and committed height).

@CHr15F0x CHr15F0x force-pushed the chris/declares-in-dummy-proposals branch 3 times, most recently from 0e2a822 to 5b39520 Compare March 12, 2026 10:05
@CHr15F0x CHr15F0x changed the title [WIP/cleanup] Chris/declares in dummy proposals test(consensus): declare HelloStarknet in the first proposal, assert transactions for reverts Mar 12, 2026
@CHr15F0x CHr15F0x marked this pull request as ready for review March 12, 2026 10:15
@CHr15F0x CHr15F0x requested a review from a team as a code owner March 12, 2026 10:15
@CHr15F0x CHr15F0x force-pushed the chris/declares-in-dummy-proposals branch 2 times, most recently from 910f36d to b71decc Compare March 12, 2026 10:43
@CHr15F0x CHr15F0x force-pushed the chris/declares-in-dummy-proposals branch from b71decc to c367a1c Compare March 12, 2026 10:45
Copy link
Contributor

@zvolin zvolin left a comment

Choose a reason for hiding this comment

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

some first pass, but haven't read through deeply yet 🙈

Copy link
Contributor

@kkovaacs kkovaacs left a comment

Choose a reason for hiding this comment

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

Looks solid other than the typos / suggested renames.

Copy link
Contributor

@t00ts t00ts left a comment

Choose a reason for hiding this comment

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

🚀

@CHr15F0x CHr15F0x force-pushed the chris/declares-in-dummy-proposals branch from 7544b27 to d0f87f5 Compare March 13, 2026 10:21
@CHr15F0x CHr15F0x merged commit 760d0f6 into main Mar 13, 2026
10 checks passed
@CHr15F0x CHr15F0x deleted the chris/declares-in-dummy-proposals branch March 13, 2026 11:16
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.

4 participants