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

Use BundleAccountLocker when handling tip txs #143

Merged
merged 23 commits into from
Sep 22, 2022
Merged

Conversation

buffalu
Copy link
Contributor

@buffalu buffalu commented Sep 20, 2022

Problem

We weren't previously locking any accounts related to processing tip receiver transactions. The consensus issues we've been seeing are what I believe to be a race condition between voting transactions that use the identity account and transactions involved with changing the tip receiver.

Summary of Changes

Use BundleAccountLocker to make sure all accounts involved in a tip transaction are locked

TODO

  • port to master
  • port to v1.11
  • test more on fleet

@buffalu buffalu force-pushed the lb/maybe_consensus_fix branch from 3602c0b to 639798b Compare September 20, 2022 17:41
read_locks: HashMap<Pubkey, u64>,
write_locks: HashMap<Pubkey, u64>,
) {
let mut read_locks_l = self.read_locks.lock().unwrap();
let mut write_locks_l = self.write_locks.lock().unwrap();
let read_locks_l = &mut self.read_locks;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit var name

working_bank: bank, ..
} = bank_start;

) -> BundleExecutionResult<Vec<SanitizedTransaction>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to wrap the return in BundleExecutionResult?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_configured_tip_receiver can return err

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually brings up another bug here that i introduced, will fix

Copy link
Contributor

Choose a reason for hiding this comment

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

the name's just confusing since no txs are really being executed here

@segfaultdoc segfaultdoc self-requested a review September 20, 2022 19:02
&read_locks,
&write_locks,
);
let batch = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

be extra safe here

f Show resolved Hide resolved
@buffalu buffalu merged commit 206c7ae into v1.10 Sep 22, 2022
@buffalu buffalu deleted the lb/maybe_consensus_fix branch September 22, 2022 19:29
buffalu pushed a commit that referenced this pull request Mar 14, 2024
…133) (#143)

* add precompile signature metrics to cost tracker (#133)

(cherry picked from commit 9770cd9)

* merge fix

* fmt

---------

Co-authored-by: Tao Zhu <[email protected]>
Co-authored-by: Tao Zhu <[email protected]>
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.

2 participants