Skip to content

Conversation

@DracoLi
Copy link
Contributor

@DracoLi DracoLi commented Nov 19, 2025

Why this should be merged

This simplifies HeightIndex implementations by removing the need to defensive copy. The current use case for database.HeightIndex should never modify the data after Put or Get, so the extra copying is unnecessary.

resolves #4453

How this works

Removed slice.Clone calls in Put and Get operations.
The database.HeightIndex interface was updated to indicate that byte slices are not safe to modify.

How this was tested

Unit tests

Need to be documented in RELEASES.md?

No

@DracoLi DracoLi marked this pull request as ready for review November 19, 2025 18:22
@DracoLi DracoLi requested review from a team, Copilot and yacovm and removed request for a team November 19, 2025 18:22
@DracoLi DracoLi changed the title refactor(heightindexdb): remove safety of byte slice modifications for Put & Get refactor(heightindex): remove safety of byte slice modifications for Put & Get Nov 19, 2025
@DracoLi DracoLi changed the title refactor(heightindex): remove safety of byte slice modifications for Put & Get refactor(heightindex): remove safety of byte modifications for Put & Get Nov 19, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the HeightIndex database interface and its implementations to remove defensive copying of byte slices, simplifying the code under the assumption that callers will not modify data after Put or Get operations.

Key Changes:

  • Removed slices.Clone calls from Put and Get operations across implementations
  • Updated interface documentation to clarify that byte slices are not safe to modify
  • Removed tests that verified defensive copying behavior

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
database/database.go Updated HeightIndex interface documentation to indicate byte slices are not safe to modify
database/heightindexdb/memdb/database.go Removed slices.Clone calls from in-memory database implementation
database/heightindexdb/dbtest/dbtest.go Removed tests verifying that returned data is independent of modifications
x/blockdb/cache_db.go Removed slices.Clone calls from cache database implementation
x/blockdb/cache_db_test.go Removed tests that verified defensive copying behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@DracoLi DracoLi force-pushed the dl/db-no-clone-get-put branch from 53f09ce to 814bd93 Compare November 21, 2025 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

(x/blockdb) Avoid cloning Get and Put data

4 participants