Skip to content

Discard packets statically known to fail#370

Merged
mergify[bot] merged 2 commits intoanza-xyz:masterfrom
apfitzge:discard_on_minimum_limit
Mar 21, 2024
Merged

Discard packets statically known to fail#370
mergify[bot] merged 2 commits intoanza-xyz:masterfrom
apfitzge:discard_on_minimum_limit

Conversation

@apfitzge
Copy link
Copy Markdown

Problem

  • Can statically determine some transactions will fail execution

Summary of Changes

  • Discard them during receiving

Fixes #

@apfitzge apfitzge requested a review from tao-stones March 21, 2024 18:04
/// This is a simple sanity check so the leader can discard transactions
/// which are statically known to exceed the compute budget, and will
/// result in no useful state-change.
pub fn compute_unit_limit_above_static_builtins(&self) -> bool {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This could be part of ImmutableDeserializedPacket::new() -> Result, can drop packet there if not enough cu limit. Just to clarify that it is standalone function to support scheduler_controller?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

made it a separate function more just more easily document the behavior and use in filter call.
also didn't want to put it directly in new so we don't need to change any of the existing tests.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (b2f4fb3) to head (65af257).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #370     +/-   ##
=========================================
- Coverage    81.9%    81.8%   -0.1%     
=========================================
  Files         838      838             
  Lines      227368   227389     +21     
=========================================
- Hits       186238   186226     -12     
- Misses      41130    41163     +33     

@apfitzge apfitzge requested a review from tao-stones March 21, 2024 19:13
Copy link
Copy Markdown

@tao-stones tao-stones left a comment

Choose a reason for hiding this comment

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

lgtm

@apfitzge apfitzge added automerge automerge Merge this Pull Request automatically once CI passes v1.17 labels Mar 21, 2024
@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 21, 2024

Backports to the stable branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule.

@mergify
Copy link
Copy Markdown

mergify Bot commented Mar 21, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

@mergify mergify Bot merged commit 5f16932 into anza-xyz:master Mar 21, 2024
mergify Bot pushed a commit that referenced this pull request Mar 21, 2024
* Discard packets statically known to fail

* add test

(cherry picked from commit 5f16932)

# Conflicts:
#	core/src/banking_stage/transaction_scheduler/scheduler_controller.rs
mergify Bot pushed a commit that referenced this pull request Mar 21, 2024
* Discard packets statically known to fail

* add test

(cherry picked from commit 5f16932)

# Conflicts:
#	core/src/banking_stage/transaction_scheduler/scheduler_controller.rs
apfitzge added a commit that referenced this pull request Mar 21, 2024
)

* Discard packets statically known to fail (#370)

* Discard packets statically known to fail

* add test

(cherry picked from commit 5f16932)

# Conflicts:
#	core/src/banking_stage/transaction_scheduler/scheduler_controller.rs

* resolve conflict

---------

Co-authored-by: Andrew Fitzgerald <apfitzge@gmail.com>
apfitzge added a commit that referenced this pull request Mar 21, 2024
)

* Discard packets statically known to fail (#370)

* Discard packets statically known to fail

* add test

(cherry picked from commit 5f16932)

# Conflicts:
#	core/src/banking_stage/transaction_scheduler/scheduler_controller.rs

* resolve conflicts: old metrics update style

---------

Co-authored-by: Andrew Fitzgerald <apfitzge@gmail.com>
@HaoranYi HaoranYi mentioned this pull request Apr 8, 2024
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
…#370) (anza-xyz#375)

* Discard packets statically known to fail (anza-xyz#370)

* Discard packets statically known to fail

* add test

(cherry picked from commit 5f16932)

# Conflicts:
#	core/src/banking_stage/transaction_scheduler/scheduler_controller.rs

* resolve conflicts: old metrics update style

---------

Co-authored-by: Andrew Fitzgerald <apfitzge@gmail.com>
OliverNChalk pushed a commit to OliverNChalk/agave that referenced this pull request Nov 11, 2025
Originally written by Andrew Fitzgerald <apfitzge@gmail.com> on Wed Aug
9 14:57:55 2023 -0700.

Previous version:

    commit 86a2b8f8aa19e606bd6396dfdbc6f35950b23ee9
    Author: Andrew Fitzgerald <apfitzge@gmail.com>
    Date:   Wed Aug 9 14:57:55 2023 -0700

        Spawn adversarial and normal banking stages (anza-xyz#113)

Rewritten to match the upstream scheduler code as of anza-xyz#5467 by Illia
Bobyr <illia.bobyr@anza.xyz>.

This change includes all of the following changes:

---

Author: Illia Bobyr <illia.bobyr@solana.com>
Date:   Mon Oct 2 14:03:46 2023 -0700

    adversary: test_scheduler => attack_scheduler (anza-xyz#175)

    We mostly talk about attacks, when we discuss the functionality this
    code supports.  Considering that we have a lot of other kinds of tests,
    it seems a bit clearer to call use "attack" in this part of the code.

Author: Illia Bobyr <illia.bobyr@solana.com>
Date:   Tue Oct 3 15:21:33 2023 -0700

    adversary: test_generators => transaction_generators (anza-xyz#178)

    We mostly talk about "attacks" rather than "tests" in this part of the
    code.  And even the main type in the `test_generators` module is called
    `TransactionGenerator`.

Author: kirill lykov <kirill.lykov@solana.com>
Date:   Thu Feb 8 10:52:48 2024 +0100

    replay: atomicbool instead of singleton for dropping packets (anza-xyz#224)

    * use atomicbool instead of singleton to drop packets

    * add use for Ordering

    Co-authored-by: Illia Bobyr <ilya.bobyr@gmail.com>
    Signed-off-by: kirill lykov <lykov.kirill@gmail.com>

    * rename drop_packets

    ---------

    Signed-off-by: kirill lykov <lykov.kirill@gmail.com>
    Co-authored-by: Illia Bobyr <ilya.bobyr@gmail.com>

Author: Brennan <brennan.watt@anza.xyz>
Date:   Fri Mar 22 06:45:29 2024 -0700

    remove dead code (anza-xyz#298)

Author: Andrew Fitzgerald <apfitzge@gmail.com>
Date:   Tue Jul 16 14:49:59 2024 -0500

    AdversarialBankingStage: Remove warning (anza-xyz#370)

    Remove warning. Adjust names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge automerge Merge this Pull Request automatically once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants