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

feat: Allow for truncation of 0x prefix hashes #61

Closed
wants to merge 3 commits into from

Conversation

SamMayWork
Copy link
Contributor

@SamMayWork SamMayWork commented Mar 7, 2024

Pre-Reqs: hyperledger/firefly-evmconnect#118

Hedera uses 48 bytes hashes for their blocks, for compatibility with FireFly we need to truncate down to 32 bytes - this is an entirely supported feature of the JSON RPC layer for Hedera, this PR adds the function for us to be able to do this.

@SamMayWork SamMayWork marked this pull request as ready for review March 25, 2024 11:52
@SamMayWork SamMayWork changed the title Add truncate method feat: Allow for truncation of 0x prefix hashes Mar 25, 2024
@@ -62,6 +62,14 @@ func (h HexBytes0xPrefix) MarshalJSON() ([]byte, error) {
return []byte(fmt.Sprintf(`"%s"`, h.String())), nil
}

func (h HexBytes0xPrefix) Truncate(length int) (HexBytes0xPrefix, error) {
if length > len(h) {
return nil, fmt.Errorf("truncation length %d larger than length of hex bytes", length)
Copy link
Contributor

@Chengxuan Chengxuan Apr 2, 2024

Choose a reason for hiding this comment

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

It's not a common behaviour that truncates throws an error when the length is smaller than the truncate size.

It seems the truncate size conveys the constraint of minSize implicitly, may worth thinking making that explicit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree - changed the logic here to return the original hex bytes if the length of the bytes is less than the desired length

Copy link
Contributor

@EnriqueL8 EnriqueL8 left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Contributor

@peterbroadhurst peterbroadhurst left a comment

Choose a reason for hiding this comment

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

@SamMayWork - could do with a little explanation of why firefly-signer common utility functions are the right place for []byte truncation functions?

@@ -62,6 +62,14 @@ func (h HexBytes0xPrefix) MarshalJSON() ([]byte, error) {
return []byte(fmt.Sprintf(`"%s"`, h.String())), nil
}

func (h HexBytes0xPrefix) Truncate(length int) HexBytes0xPrefix {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what this PR does sorry @SamMayWork

It seems like this is just a bytes manipulation function - surely that could just be done directly by the calling function?
Why is this a helper that is unique to the HexBytes0xPrefix type wrapper for String() formatting?

@SamMayWork
Copy link
Contributor Author

Not required.

@SamMayWork SamMayWork closed this May 10, 2024
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.

4 participants