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

Small fixes in soroban-simulation #1361

Merged
merged 1 commit into from
Mar 1, 2024
Merged

Conversation

dmkozh
Copy link
Contributor

@dmkozh dmkozh commented Mar 1, 2024

What

  • Use &impl SnapshotSourceWithArchive in load_from_snapshot. This is not very pretty semantically, but there is no clean way for storage to implement archived entry filtering without accessing the config and I really want to encapsulate the config-related logic in NetworkConfig struct. The alternative would be for SnapshotSource implementer to either load the state expiration config before building the config, or to not follow the interface contract of SnapshotSource to not ever return expired entries.
  • Use &self in fill_config_fields_in_ledger_info

Why

Issue fixes

Known limitations

N/A

- Use `&impl SnapshotSourceWithArchive` in `load_from_snapshot`. This is not very pretty semantically, but there is no clean way for storage to implement archived entry filtering without accessing the config and I really want to encapsulate the config-related logic in `NetworkConfig` struct. The alternative would be for `SnapshotSource` implementer to either load the state expiration config before building the config, or to not follow the interface contract of `SnapshotSource` to not ever return expired entries.
- Use `&self` in `fill_config_fields_in_ledger_info`
@dmkozh dmkozh requested a review from a team as a code owner March 1, 2024 02:02
@dmkozh dmkozh added this pull request to the merge queue Mar 1, 2024
Merged via the queue into stellar:main with commit 8c9ab83 Mar 1, 2024
13 checks passed
@dmkozh dmkozh deleted the snapshot_followup branch March 1, 2024 17:43
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