Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Open
1 of 4 tasks
Tracked by #4128
anupsdf opened this issue May 14, 2024 · 1 comment
Open
1 of 4 tasks
Tracked by #4128

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

anupsdf opened this issue May 14, 2024 · 1 comment
Assignees
@marta-lokhova

Comments

@anupsdf
Copy link
Contributor

anupsdf commented May 14, 2024 •
edited by marta-lokhova
Loading

@anupsdf anupsdf mentioned this issue May 14, 2024
Open
11 tasks
@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
@marta-lokhova marta-lokhova mentioned this issue Sep 18, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Oct 16, 2024
@marta-lokhova
Cleanup apply and validation flows (#4471)
bcfbbe4 
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`
@marta-lokhova marta-lokhova mentioned this issue Dec 19, 2024
Open
4 tasks
@marta-lokhova marta-lokhova mentioned this issue Jan 3, 2025
Merged
github-merge-queue bot pushed a commit that referenced this issue Jan 3, 2025
@SirTyson
Make read-only ledger state const (#4602)
7a4110a 
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
@marta-lokhova
Copy link
Contributor

Merge after #4316 is done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marta-lokhova marta-lokhova

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@marta-lokhova @anupsdf and others