Move validator metadata DB writes into validatorState#5008
Open
Move validator metadata DB writes into validatorState#5008
Conversation
DracoLi
force-pushed
the
dl/pchain-validator-write
branch
from
d388bae to
96a1cda
Compare
…rState - Add AddValidatorMetadata to register and mark new entries for writing - Track pending deletes in DeleteValidatorMetadata for batch processing - Remove direct DB writes from writeCurrentStakers
DracoLi
force-pushed
the
dl/pchain-validator-write
branch
from
96a1cda to
fe7d4cc
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors validator metadata database operations to consolidate all writes within the validatorState interface, improving code organization and separation of concerns. Previously, metadata DB writes were split between writeCurrentStakers and WriteValidatorMetadata, making the flow harder to follow.
Changes:
- Added
AddValidatorMetadatamethod to stage new validator metadata for persistence - Enhanced
DeleteValidatorMetadatato track pending delete operations - Moved all metadata DB write logic into
WriteValidatorMetadata, which now processes deletes before puts - Updated
WriteValidatorMetadatato acceptKeyValueWriterDeleterinstead ofKeyValueWriterto support delete operations
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| vms/platformvm/state/state.go | Refactored writeCurrentStakers to use AddValidatorMetadata for adds and moved WriteValidatorMetadata call to the end of the function; removed direct DB operations |
| vms/platformvm/state/metadata_validator.go | Added AddValidatorMetadata method, updated DeleteValidatorMetadata to track pending deletes, and enhanced WriteValidatorMetadata to process all DB operations (deletes first, then puts) |
| vms/platformvm/state/metadata_validator_test.go | Added comprehensive tests for the new AddValidatorMetadata method and various delete scenarios (delete after write, add-then-delete, delete-then-re-add) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this should be merged
Validator metadata DB writes (puts and deletes) were split between
writeCurrentStakersandWriteValidatorMetadata, making the mutation logic harder to follow. This consolidates all DB writes intovalidatorState, sowriteCurrentStakersfirst stages changes andWriteValidatorMetadatahandles all persistence in one place.How this works
Add
AddValidatorMetadatato thevalidatorStateinterface, which registers new metadata and marks it dirty for the next write.DeleteValidatorMetadatanow also tracks pending deletes. WriteValidatorMetadata processes deletes before puts and acceptsKeyValueWriterDeleterinstead ofKeyValueWriter.How this was tested
New unit tests
Need to be documented in RELEASES.md?
No