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.

Sign up for GitHub

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

Jump to bottom

hexToBytes should accept and trim strings with leading 0x #171

Open
whoabuddy opened this issue Jan 20, 2023 · 3 comments · May be fixed by #172
Open

hexToBytes should accept and trim strings with leading 0x #171

whoabuddy opened this issue Jan 20, 2023 · 3 comments · May be fixed by #172

Comments

@whoabuddy
Copy link

This started with testing a simple Clarity contract that can stack its own STX through pool delegation.

The Short Story

Would there be an issue if the hexToBytes function were to check if the string starts with 0x, and remove it before processing the string?

Happy to submit a PR! 🚀

The Long Story

There were two function parameters that needed to be (buff 1) and (buff 20) to represent the pox address in delegate-stx.

The hex values were each stored in a constant:
(incorrect code below for illustration)

const poxVer = "0x01";
const poxHash = "0x13effebe0ea4bb45e35694f5a15bb5b96e851afb";

My first instinct was to use bufferCVFromString() on these two values, but the buffers were not the correct length as they were literally processing the string and returning (buff 3) and (buff 40). This made makeContractCall() fail with the tx options.

Next step was @friedger's suggestion to use bufferCV() and hexToBytes(), and after passing the string the contract call would be created but the broadcastTransaction() step would fail due to BadFunctionArguments.

After updating the constant to remove the leading 0x everything worked as expected, but it was quite the fight to get there and identify each problem.
(correct code below)

const poxVer = "01";
const poxHash = "13effebe0ea4bb45e35694f5a15bb5b96e851afb";

Relevant TX:
https://explorer.stacks.co/txid/0xdbbecf8113b530bd12ca67c425cc1c27a0bf7916ded65e106cf0ecc8fb28c75a?chain=mainnet
@aulneau
Copy link
Contributor

aulneau commented Jan 20, 2023

yeah would be helpful! please feel free to create a pr :)

This was referenced Jan 20, 2023
Closed
Open
Open
@whoabuddy whoabuddy changed the title hexToString should accept and trim strings with leading 0x hexToBytes should accept and trim strings with leading 0x Jan 27, 2023
@whoabuddy
Copy link
Author

Based on the discussion in hirosystems/clarinet#786 and my notes in the original issue, I think it'd be nice if bufferCVFromString() would check for and pass along the hex string as-is.

Maybe something like this in bufferCV.ts? Can add to the open PR.

export const bufferCVFromString = (string: string): BufferCV =>
  string.startsWith("0x")
    ? bufferCV(hexToBytes(string))
    : bufferCV(utf8ToBytes(string));

@friedger
Copy link

If we propose using 0x as marker for hex encoding, we should be consistent and do it all over the library and not allow non prefixed hex strings.

For the developers, it is confusing how to handle hex strings.

I'd propose bufferCVFromHexString

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add check for 0x in hex string whoabuddy/micro-stacks
3 participants
@friedger @whoabuddy @aulneau