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(consensus): resubmit gc-transaction #5707

Conversation

VorobyevIlya
Copy link
Contributor

@VorobyevIlya VorobyevIlya commented Mar 2, 2025

Porting upstream change: MystenLabs/sui@ae6275f

Description of change

Copied from upstream:
'Transactions of GC'ed blocks should be retried to remain compliant with
the so far system assumptions. Refactored the consensus side to signal
back the submitter about the status of the block that the submitted
transaction has been included to. Practically two possible outcomes:

  1. Sequenced (when the block has been committed and consequently the tx
    as well)
  2. GarbageCollected (when the tx's block has not been committed and
    passed gc_round)
    also took the opportunity for a small refactoring around the
    SubmitToConsensus trait and the ConsensusClient as intentions
    started diverging.'

There is a difference from upstream changes in consensus_tests.rs, since some changes in upstream depend on previous commit f6dd9a8 that we haven't ported yet. So these changes will be ported later.

Links to any relevant issues

Fixes #5643

Type of change

Choose a type of change, and delete any options that are not relevant.

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How the change has been tested

Describe the tests that you ran to verify your changes.

Make sure to provide instructions for the maintainer as well as any relevant configurations.

Change checklist

Tick the boxes that are relevant to your changes, and delete any items that are not.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that new and existing unit tests pass locally with my changes

Copy link

vercel bot commented Mar 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
apps-backend ⬜️ Ignored (Inspect) Visit Preview Mar 5, 2025 11:01pm
apps-ui-kit ⬜️ Ignored (Inspect) Visit Preview Mar 5, 2025 11:01pm
rebased-explorer ⬜️ Ignored (Inspect) Visit Preview Mar 5, 2025 11:01pm
wallet-dashboard ⬜️ Ignored (Inspect) Visit Preview Mar 5, 2025 11:01pm

@iota-ci iota-ci added consensus Issues related to the Core Consensus team core-protocol labels Mar 2, 2025
@VorobyevIlya VorobyevIlya force-pushed the consensus/feat/resubmit-gc-transaction branch from 1f8d83d to 4ff79fe Compare March 3, 2025 13:58
@VorobyevIlya VorobyevIlya self-assigned this Mar 3, 2025
@VorobyevIlya VorobyevIlya marked this pull request as ready for review March 4, 2025 08:52
@VorobyevIlya VorobyevIlya requested review from a team as code owners March 4, 2025 08:52
@piotrm50 piotrm50 force-pushed the consensus/feat/DagState-to-evict-blocks-based-on-GC-round branch from 91651f6 to 2418e7f Compare March 5, 2025 00:14
@piotrm50 piotrm50 requested review from a team as code owners March 5, 2025 00:14
@VorobyevIlya VorobyevIlya marked this pull request as draft March 5, 2025 15:00
@piotrm50 piotrm50 force-pushed the consensus/feat/resubmit-gc-transaction branch from e76e3a2 to 13b74ae Compare March 5, 2025 18:12
Copy link
Contributor

@piotrm50 piotrm50 left a comment

Choose a reason for hiding this comment

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

many changes missing

@piotrm50 piotrm50 force-pushed the consensus/feat/DagState-to-evict-blocks-based-on-GC-round branch from 50fcc16 to 45d759f Compare March 5, 2025 19:25
Base automatically changed from consensus/feat/DagState-to-evict-blocks-based-on-GC-round to consensus/feat/garbage-collector March 5, 2025 19:31
@VorobyevIlya VorobyevIlya force-pushed the consensus/feat/resubmit-gc-transaction branch from 13b74ae to 240967f Compare March 5, 2025 19:54
@VorobyevIlya
Copy link
Contributor Author

many changes missing

@piotrm50 I initially ignored the changes outside of consensus crate, since they were dependent on previous commit f6dd9a8 we didn't port. Now I took them. The difference turned out to be quite small and located in consensus_tests.rs file

@VorobyevIlya VorobyevIlya requested a review from piotrm50 March 6, 2025 08:01
@VorobyevIlya VorobyevIlya marked this pull request as ready for review March 6, 2025 08:02
@VorobyevIlya VorobyevIlya marked this pull request as draft March 6, 2025 09:53
@VorobyevIlya VorobyevIlya marked this pull request as ready for review March 6, 2025 12:27
@alexsporn alexsporn added this to the v0.12.x - protocol v6 milestone Mar 6, 2025
@VorobyevIlya VorobyevIlya merged commit 329c17a into consensus/feat/garbage-collector Mar 7, 2025
98 of 102 checks passed
@VorobyevIlya VorobyevIlya deleted the consensus/feat/resubmit-gc-transaction branch March 7, 2025 18:43
VorobyevIlya added a commit that referenced this pull request Mar 10, 2025
* resubmit gc-transaction

* fix formatting

* remove dead code

* update SubmitToConsensus and ConsensusClient

* fix tests amd metrics

* fix formatting
VorobyevIlya added a commit that referenced this pull request Mar 10, 2025
* resubmit gc-transaction

* fix formatting

* remove dead code

* update SubmitToConsensus and ConsensusClient

* fix tests amd metrics

* fix formatting
piotrm50 pushed a commit that referenced this pull request Mar 14, 2025
* resubmit gc-transaction

* fix formatting

* remove dead code

* update SubmitToConsensus and ConsensusClient

* fix tests amd metrics

* fix formatting
piotrm50 pushed a commit that referenced this pull request Mar 17, 2025
* resubmit gc-transaction

* fix formatting

* remove dead code

* update SubmitToConsensus and ConsensusClient

* fix tests amd metrics

* fix formatting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus Issues related to the Core Consensus team core-protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants