Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Add method for stripping leading hex zeroes #79

Closed
danfinlay opened this issue May 30, 2017 · 10 comments
Closed

Add method for stripping leading hex zeroes #79

danfinlay opened this issue May 30, 2017 · 10 comments

Comments

@danfinlay
Copy link

Opening for comment.

Geth recently became more strict and now rejects many hex values that have leading zeroes (ie 0x02 instead of 0x2).

The official RPC spec dictates that quantities should have no leading zero, and unformatted data should.

For this reason, it would probably make sense to include some new methods here for creating un-prefixed hex numbers.

I'm curious what kind of API surface people would like for that. Maybe something as simple as a stripZeroPrefix() method? I could submit that PR in minutes. We could also have a few new methods, for converting different things to this new type of hex, but that might get more confusing.

Thoughts? Would a stripZeroPrefix() method get merged?

@axic
Copy link
Member

axic commented Feb 4, 2018

How about something along the lines of shortestHexRepresenation?

@Neurone
Copy link
Contributor

Neurone commented Jul 5, 2018

Correct handling of quantities and unformatted data is already implemented by ganache-core. I think it is more correct to have those features there because ganache-core is specifically about handling RPC communications.

Maybe those features could be more useful into ethrpc, though reading the project's description it seems they are already there. I didn't find them but notice the output of rpc.eth.blockNumber().

@holgerd77
Copy link
Member

This is not about correct handling quantities and data but just about providing a helper function which would transform data into a specific format (which could be e.g. used for correct data transmission in the problem case described above).

Don't see why this shouldn't fit into the scope of this library. I actually prefer the stripZeroPrefix() naming since this is more explicit about what it does without leaving anything out, shortestHexRepresentation() is less easy to directly grab the functionality.

@Neurone
Copy link
Contributor

Neurone commented Jul 6, 2018

My reasoning is this.

  • If we want to comply with RPC spec, we must handle data and quantities in correct ways as ganache-core: stripping the prefixed zero is not enough;
  • If we don't want to comply with RPC spec, I cannot see any other reasons to have a function to remove the leading 0; I think this can also be misleading and it can bring people to think they can actually comply with RPC spec simply using stripZeroPrefix();

@holgerd77
Copy link
Member

@Neurone Hmm, this is not specifically for RPCbut can be used in different contexts?

@Neurone
Copy link
Contributor

Neurone commented Jul 10, 2018

@holgerd77 Sorry, but I currently cannot see the usefulness of this in other contexts. The stripZero() function should be enough for anyone that wants only to remove leading zeros.

@whymarrh
Copy link
Contributor

whymarrh commented Nov 29, 2018

There is prior art for having a strip/trim function that accepts a direction (defaulting to some direction, probably trailing) e.g. Erlang[1], Matlab[2], and R[3]. We could introduce a stripZero flag that allows us to handle both cases.

Edit: Looks like there's already precedent for that kind of API here in setLength:

ethereumjs-util/index.js

Lines 97 to 122 in ff7bb86

/**
* Left Pads an `Array` or `Buffer` with leading zeros till it has `length` bytes.
* Or it truncates the beginning if it exceeds.
* @method setLengthLeft
* @param {Buffer|Array} msg the value to pad
* @param {Number} length the number of bytes the output should be
* @param {Boolean} [right=false] whether to start padding form the left or right
* @return {Buffer|Array}
*/
exports.setLengthLeft = exports.setLength = function (msg, length, right) {
const buf = exports.zeros(length)
msg = exports.toBuffer(msg)
if (right) {
if (msg.length < length) {
msg.copy(buf)
return buf
}
return msg.slice(0, length)
} else {
if (msg.length < length) {
msg.copy(buf, length - msg.length)
return buf
}
return msg.slice(-length)
}
}

@holgerd77
Copy link
Member

I am actually getting lost here a bit what trim/strip cases we are talking about and what cases are not covered by actual functionality like unpad.

Could you give 4-5 practical in-out examples together with the flag you are suggesting?

@whymarrh
Copy link
Contributor

whymarrh commented Mar 6, 2019

This is offered by unpad:

addHexPrefix(unpad('0x00060a24181e4000')) === '0x60a24181e4000'
// => true

@holgerd77
Copy link
Member

Will close in favor of ethereumjs/ethereumjs-monorepo#1302

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants
@axic @danfinlay @Neurone @holgerd77 @whymarrh and others