Merkle root validation fails with additional tx input

[unlimit]

Hi!
I noticed that python and js versions differently compare Merkle root.
In JS validation is more strict and if your transaction's input has additional data - validation fails.
But it will pass in python.

Is additional tx input allowed?

[kimdhamilton]

Hi,
Thank you for finding and raising this issue. Just to confirm -- it sounds like a transaction with >1 tx input causes Blockcerts verification to fail?

The reason I ask is that an additional tx input should not cause the Merkle root validation to fail; those are separate steps. The Merkle root validation is comparing the OP_RETURN (or ETH_DATA, etc) field, which is an output and not an input.

Either way, addressing the issue about >1 inputs directly, while the Blockcerts standard does not specifically call out this case, there is no reason to disallow it. In fact, if it is failing, we should clarify this cause in the spec and fix this library.

Let's keep this issue open so we can investigate more to see how/why it is failing and fix as necessary.

Thanks,
Kim

[unlimit]

Unlike Bitcoin, Ethereum doesn't have OP_RETURN. Only Input Data field can be used for Merkle root validation.
Lets compare a few different Ethereum transaction:
1 This TX , provided in docs is direct ETH transfer with only one input - merkle root
2. This TX is an example of smart contract call. Input Data here contains:

1. Smart contract method ID
2. Params, passed to this method. One of params is Merkle root (378e3e307a13df5fda84987df80bb9c130bdad8a539146dccfa4b543b08322de)

Thats why comparison should not be strict!

[unlimit]

Any updates for this issue?

[kimdhamilton]

@unlimit can you update the links? The first link goes to the cert-verifier-js github repo, and I think the second is what you meant to show for the first.

Ours do not have smart contract method ids, so I want to make sure I understand what you are referring to. For reference, here is an example of a ethereum-issued Blockcert: https://etherscan.io/tx/0xa12c498c8fcf59ee2fe785c94c38be4797fb027e6450439a7ef30ad61d7616d3

I could be missing something; @AnthonyRonning can you weigh in on this when you get a chance?

I'm not seeing the merle root string 378e3e307a13df5fda84987df80bb9c130bdad8a539146dccfa4b543b08322de in the transaction you linked @unlimit .

Maybe a copy paste error, but essentially, I think we assume that the only data in the input is going to be the merkle root. You essentially want to send this merkle root to a smart contract, and do something like a string.contains() on the input data, to see if the Merkle root exists anywhere in the data, and mark that as good?

[unlimit]

@kimdhamilton , yes the first link is here
@AnthonyRonning input data for second link contains 3 params(united into one string). Last param is merkle root. I made it bold, see below

0x1908999f000000000000000000000000959fd7ef9089b7142b6b908dc3a8af7aa8ff0fa1378e3e307a13df5fda84987df80bb9c130bdad8a539146dccfa4b543b08322de

@unlimit oh sorry, it looks like my ctrl-f wasn't picking it up.

I don't see too much of a problem parsing the entire string to see if it contains a substring relating to the merkle root. I think when we were prototyping segwit transactions, that had to be the case as well.

[unlimit]

Updated pr #45

[guix77]

If I understand well, the OP wanted in 2018 to replace the usual burn transaction by a more global Eth transaction with additional data, so in #45 we would have had to regex to get the Merkle tree root hash for Blockcerts.

In the light of the upcoming Blocerts 3 I would suggest to close this issue. @unlimit where are you at with this?

[unlimit]

If v3 allows to issue valid certificate by a SmartContract method, let's close this issue.

[guix77]

Sorry, I might have suggested wrongly to close this, because it would have to be seen if v3 would allow to issue through a smart contract. What I've already deduced is that v3 could - but it's not planned for now at all - use a smart contract to build a trusted registry of issuers. But it's not the same.

[lemoustachiste]

This is not part of the implementation of blockcerts v3, so this could still be a valid case.

I didn't follow this issue at the beginning and I'm trying to wrap my head around the implications.

I believe the odds of having the Merkle root matching piece of string anywhere else in the input data would be low, but, since we are changing the what the input data could be and don't control its length anymore, couldn't that open a security weakness?

I would be more comfortable strengthening the check:

• by ensuring that the regex checks that the string ends with the Merkle root
• if possible (as in if we can predict it) make a check for the expected length of the previous data.

@unlimit wdyt?