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

TransactionByHash failing due to inability to parse txdata.S value #19461

Closed
Talvish opened this issue Apr 14, 2019 · 11 comments
Closed

TransactionByHash failing due to inability to parse txdata.S value #19461

Talvish opened this issue Apr 14, 2019 · 11 comments
Labels

Comments

@Talvish
Copy link

Talvish commented Apr 14, 2019

I have some ethereum code running in a contract that works fine. The code performs a mint operation of a type of token I created. The contract is currently compiled using geth but the contracts are deployed into Ganache (and I saw the following issue regardless of running against Ganache 2.0.0 and 1.2.2).

Today I updated my golang code to monitor and validate the mint transactions (mainly to see if they were pending) using TransactionByHash. What I found was that this call will periodically fail on some transactions with the message 'json: cannot unmarshal hex number with leading zero digits into Go value of type *hexutil.Big'.

After some extra logging in the go-ethereum code I produced the following output:


2019/04/13 23:07:50 RAW TRANSACTION: {"hash":"0x8d277a01e76a71928407a03b95daf9125d23cb600cbca15e04a5a7e4f13df26f","nonce":"0x53","blockHash":"0xad31d7f6b3c9650ed097a51e60ef09bb892e4e3b670fd892bebab6e78d3b339d","blockNumber":"0x54","transactionIndex":"0x0","from":"0xe26abae657fea0d35a1880520aeb23eab3a65fef","to":"0xcb8dc2a714624ede9ce2e585e0208aa427f8df6b","value":"0x0","gas":"0x4c4b40","gasPrice":"0x4a817c800","input":"0x40c10f190000000000000000000000000816ec558ade35878385b8a596e5a6e1e945fdf600000000000000000000000000000000000000000000000000000069d50535d1","v":"0x1c","r":"0x6e2345ffa0b1c7eacc66962757969d6ed940a729dca65431b1eb4dee51f470a9","s":"0x0528e7f3def038c34b93cd32a55610d10afaed61bfa18e35e364797343a4e07b"}
2019/04/13 23:07:50 FAILED VALUE IS: 0528e7f3def038c34b93cd32a55610d10afaed61bfa18e35e364797343a4e07b
2019/04/13 23:07:50 unable to get transaction for hash '0x8d277a01e76a71928407a03b95daf9125d23cb600cbca15e04a5a7e4f13df26f' due to error [json: cannot unmarshal hex number with leading zero digits into Go value of type *hexutil.Big]

What you can see from the above is that the 's' value in the transaction isn't parsing because the 's' value is being unmarshalled as a hexutil.Big which uses 'checkNumberText' BUT that function fails due to the starting 0.

So while my operations are correct and things are minted fine, I have no way to verify the operation isn't pending (so I can potentially resubmit, etc).

System information

Geth version: 1.9.0-unstable (though failed on earlier version as well)
OS & Version: Windows 10 / WSL using Ubuntu 18.04

Expected behaviour

The TransactionByHash should load the otherwise successful transactions.

Actual behaviour

The transaction isn't loading due to a failure (see above)

Steps to reproduce the behaviour

Hard to say without using my code explicitly, but I am easily able to reproduce by attempting to load and check the mint transactions. I programmatically deploy multiple of the contract and then call mint on each. In any grouping of 4 calls I seem to guarantee at least 1 failure.

Backtrace

@Talvish
Copy link
Author

Talvish commented Apr 14, 2019

For a test I commented out the line in checkNumbetText (found in the file json.go) that would fail if there was a leading 0 and at least on the surface level there were no adverse issues and I was able to load the transaction:

func checkNumberText(input []byte) (raw []byte, err error) {
	if len(input) == 0 {
		return nil, nil // empty strings are allowed
	}
	if !bytesHave0xPrefix(input) {
		return nil, ErrMissingPrefix
	}
	input = input[2:]
	if len(input) == 0 {
		return nil, ErrEmptyNumber
	}
	if len(input) > 1 && input[0] == '0' {
		// NOTE: COMMENTING OUT THE LINE BELOW AS A TEST
		// return nil, ErrLeadingZero
	}
	return input, nil
}

@karalabe
Copy link
Member

Hmm, apparently we interpret the signature R, S, V fields are numbers, not as binary blobs. Numbers cannot have leading zeroes. Not sure what the correct representation is, perhaps that should be answered first, then we can decide who's at fault. Ping @fjl

@Talvish
Copy link
Author

Talvish commented Apr 18, 2019

Thanks for the response. Hopefully some insights soon. At some point I'll be switching to test against Geth instead of Ganache but local development we use Ganache.

Note: the version of Ganache this was against was 2.0.0. I'll be trying again against 2.0.1 (which was just released) shortly.

@Talvish
Copy link
Author

Talvish commented Apr 24, 2019

Quick update: When using Ganache 2.0.1 there is the same issue. I'm still seeing txdata.S values that start with a zero.

@Talvish
Copy link
Author

Talvish commented May 3, 2019

Just wanted to see if there was any update here?

@fjl
Copy link
Contributor

fjl commented Jun 7, 2019

This is a bug in ganache. Transaction signature values returned through RPC should not contain leading zero.

@fjl fjl closed this as completed Jun 7, 2019
@fjl fjl reopened this Jun 7, 2019
@fjl
Copy link
Contributor

fjl commented Jun 7, 2019

Sorry, my bad. I re-checked the RPC docs and R, S values are of type DATA, which means leading zeros are allowed after all.

@fjl
Copy link
Contributor

fjl commented Jun 7, 2019

We should fix ganache anyway and update the spec. Transaction R, S values are integers everywhere. Parity treats them as integers also.

@fjl
Copy link
Contributor

fjl commented Jun 11, 2019

@fjl
Copy link
Contributor

fjl commented Jun 11, 2019

Ganache PR: trufflesuite/ganache#432

@fjl
Copy link
Contributor

fjl commented Jun 11, 2019

Canonical issue for this is now: #18152

@fjl fjl closed this as completed Jun 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants