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

refactor: replace kv-store backend in pxe, key store and wallet #12087

Merged
merged 14 commits into from
Feb 20, 2025

Conversation

alexghr
Copy link
Contributor

@alexghr alexghr commented Feb 18, 2025

Fix #11658

@alexghr alexghr added e2e-all CI: Enables this CI job. network-all Run this CI job. labels Feb 18, 2025
@alexghr alexghr requested a review from Thunkar February 19, 2025 16:00
Comment on lines +26 to +29
auto code = call_lmdb_func_with_return(mdb_put, tx.underlying(), db.underlying(), &dbKey, &dbVal, flags);
if (code == MDB_KEYEXIST && duplicatesPermitted) {
return;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pointing out this change since I encountered instances in this refactor where the same key/value entry was being added to a dupSort database.

This change makes the C++ module swallow the error making it behave more like the previous wrapper.

Copy link
Contributor

@Thunkar Thunkar left a comment

Choose a reason for hiding this comment

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

LGTM! At least on the ts side 😅


private static instance: WalletDB;

private aliasCache = new Map<string, string>();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This cache is necessary because the parsers for custom Commander options need to by synchronous.

Copy link
Collaborator

@PhilWindle PhilWindle left a comment

Choose a reason for hiding this comment

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

Looks ok to me but should probably ask @Thunkar to take a look.

@PhilWindle
Copy link
Collaborator

Looks ok to me but should probably ask @Thunkar to take a look.

Hah, looks like he already did!

@alexghr alexghr merged commit b088422 into master Feb 20, 2025
12 checks passed
@alexghr alexghr deleted the ag/refactor-pxe-db branch February 20, 2025 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
e2e-all CI: Enables this CI job. network-all Run this CI job.
Projects
None yet
3 participants