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

background ledger close: cleanup core interfaces and add invariance (tech debt) #4318

Open
1 of 4 tasks
Tracked by #4128
anupsdf opened this issue May 14, 2024 · 0 comments
Open
1 of 4 tasks
Tracked by #4128
Assignees

Comments

@anupsdf
Copy link
Contributor

anupsdf commented May 14, 2024

@marta-lokhova marta-lokhova changed the title background ledger close: overhaul of core interfaces. Right now core classes are lacking proper encapsulation and separation of responsibilities (e.g. many classes have access to Application class, which allows them to get and set a bunch of state). This makes synchronization much harder to get right. background ledger close: overhaul of core interfaces (tech debt) May 14, 2024
@marta-lokhova marta-lokhova self-assigned this Jul 9, 2024
github-merge-queue bot pushed a commit that referenced this issue Oct 16, 2024
Part of #4318 - main
change is to stop passing around Application during validation and
application flows, and use AppConnector instead, which is forced to
either assert main thread or implement thread-safe methods

Resolves #3800 - note that
READ_ONLY_WITHOUT_SQL_TXN LedgerTxn mode should go away completely once
we switch to mandatory BucketListDB. It looks like we were using RO
LedgerTxn during apply scenarios, which I think is not legal? Either
way, all READ_ONLY_WITHOUT_SQL_TXN usages except for the legacy one in
LedgerSnapshot have been removed now.

Note that the second commit has a huge diff but is a no-op, as it's
basically a find and replace for `Application -> AppConnector`
github-merge-queue bot pushed a commit that referenced this issue Jan 3, 2025
Improve access to ledger state, to better support parallelization
changes in #4543

Note that management of SorobanNetworkConfig is still not great, as
currently LM manages multiple versions of the config. Ideally, soroban
network config lives inside of the state snapshot (either BucketList
snapshot or LedgerTxn), but this was too tricky to implement at this
time due to how network config is currently implemented. We may need to
clean this up later.

This change also partially addresses
#4318
@marta-lokhova marta-lokhova changed the title background ledger close: overhaul of core interfaces (tech debt) background ledger close: cleanup core interfaces and add invariance (tech debt) Jan 10, 2025
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

No branches or pull requests

3 participants
@marta-lokhova @anupsdf and others