Skip to content

v2.2: Do not insert into accounts lt hash cache if freeze has started (backport of #6307)#6320

Merged
brooksprumo merged 2 commits into
v2.2from
mergify/bp/v2.2/pr-6307
May 28, 2025
Merged

v2.2: Do not insert into accounts lt hash cache if freeze has started (backport of #6307)#6320
brooksprumo merged 2 commits into
v2.2from
mergify/bp/v2.2/pr-6307

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented May 27, 2025

Problem

When activating the accounts lt hash feature (SIMD-215) on testnet, there was a bank hash mismatch caused by a bad lt hash.

After investigating with @jstarry1, it was concluded this is possible when the leader is still executing transactions after bank freeze has started. This would allow putting an entry into the accounts lt hash cache that was not the initial value of the account, and instead the value after bank freeze finishes up any deferred changes to account state. Then, then calculating the lt hash for the slot, we'd compute the wrong final lt hash value, since the initial state of this account was incorrect in the cache.

Here's an example:

  1. A bank calls freeze() and begins finishing up deferred changes to account state.
  2. One of ☝️ is distributing transaction fees, and that stores updates to account A.
  3. Concurrently, a transaction T is executing that includes an instruction that modifies account A.
  4. T loads A, and as part of loading A, we call inspect_account(). The value of A that gets loaded is after step (2). The modified A is added to the accounts lt hash cache.
  5. Transaction T finished executing and sees it is past the end of the block so it does not commit.
  6. When calculating the accounts lt hash, we see that A is a modified account in this slot because of step (2). As part of subtracting out the old state of account A, we look at the accounts lt hash cache and see there is an entry for A. However, this version of A is the same as the modified version, and not the old version of A. Thus, we calculate the accounts lt hash value.

Summary of Changes

Disallow adding entries to the accounts lt hash cache if freeze has started.

Justification to Backport

The PR is a patch to fix a bug that can cause a leader to compute the wrong lattice hash for a slot. This results in a bank hash mismatch, which we'd like to avoid.


This is an automatic backport of pull request #6307 done by [Mergify](https://mergify.com).

Footnotes

  1. https://discord.com/channels/428295358100013066/838890116386521088/1375339076370300958

@mergify mergify Bot requested a review from a team as a code owner May 27, 2025 14:57
@brooksprumo brooksprumo requested review from HaoranYi and jstarry May 27, 2025 15:23
Copy link
Copy Markdown

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 27, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.4%. Comparing base (ef2a0f3) to head (d107da6).
⚠️ Report is 38 commits behind head on v2.2.

Additional details and impacted files
@@            Coverage Diff            @@
##             v2.2    #6320     +/-   ##
=========================================
- Coverage    83.4%    83.4%   -0.1%     
=========================================
  Files         806      806             
  Lines      373321   373344     +23     
=========================================
- Hits       311564   311467     -97     
- Misses      61757    61877    +120     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@brooksprumo brooksprumo merged commit 9429636 into v2.2 May 28, 2025
46 checks passed
@brooksprumo brooksprumo deleted the mergify/bp/v2.2/pr-6307 branch May 28, 2025 16:42
nibty pushed a commit to x1-labs/tachyon that referenced this pull request Jun 11, 2025
… (backport of anza-xyz#6307) (anza-xyz#6320)

* Do not insert into accounts lt hash cache if freeze has started (anza-xyz#6307)

(cherry picked from commit 273f713)

* fix build due to sdk dependency renames

---------

Co-authored-by: Brooks <brooks@anza.xyz>
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.

5 participants