pokt-network / pocket

Official implementation of the Pocket Network Protocol v1
https://pokt.network
MIT License
61 stars 33 forks source link

[Persistence] Update KVStore to use Badger wrapper functions #838

Closed h5law closed 1 year ago

h5law commented 1 year ago

Description

This PR introduces unit tests to cover the KVStore's functionality as well as updating the KVStore logic to use the Badger wrapper functions update, view, etc.

This fixes the issues around deleting keys from the KVStore.

Issue

Fixes N/A

Type of change

Please mark the relevant option(s):

List of changes

Testing

Required Checklist

If Applicable Checklist

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 77.08% and project coverage change: +0.33 :tada:

Comparison is base (2d4f789) 31.52% compared to head (9198c7a) 31.86%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #838 +/- ## ========================================== + Coverage 31.52% 31.86% +0.33% ========================================== Files 107 108 +1 Lines 9034 9126 +92 ========================================== + Hits 2848 2908 +60 - Misses 5846 5874 +28 - Partials 340 344 +4 ``` | [Impacted Files](https://app.codecov.io/gh/pokt-network/pocket/pull/838?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pokt-network) | Coverage Δ | | |---|---|---| | [persistence/kvstore/kvstore.go](https://app.codecov.io/gh/pokt-network/pocket/pull/838?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pokt-network#diff-cGVyc2lzdGVuY2Uva3ZzdG9yZS9rdnN0b3JlLmdv) | `65.21% <77.08%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

h5law commented 1 year ago

NOTE @Olshansk, @dylanlott is currently working on savepoints/rollbacks that touches the same code and this PR will probably only be used as a reference as he is changing the logic their. The new wrapper functions fix the bug with Delete() but need to be integrated with savepoints. Will wait on merging this until @dylanlott gives an update

Olshansk commented 1 year ago

This PR is already complete and ready to be merged in, while savepoints & rollbacks PR is 1+ weeks away.

IMO we should merge this in and iterate on top of it. However, I will defer to @dylanlott to decide.

dylanlott commented 1 year ago

This PR is already complete and ready to be merged in, while savepoints & rollbacks PR is 1+ weeks away.

IMO we should merge this in and iterate on top of it. However, I will defer to @dylanlott to decide.

I agree with merging this and addressing the logic changes necessary for savepoints later. It shouldn't hold this up.