Skip to content
This repository has been archived by the owner on Apr 6, 2020. It is now read-only.

EIP-155 support/no support use case questions #152

Closed
holgerd77 opened this issue May 13, 2019 · 6 comments
Closed

EIP-155 support/no support use case questions #152

holgerd77 opened this issue May 13, 2019 · 6 comments

Comments

@holgerd77
Copy link
Member

Reposted from @rajeshsubhankar comment in #151:

I have 2 use cases where I struggled to understand how the library works.

Use case 1 (no EIP-155 support):

I want to get the rlp encoded serialized transaction data without v,r,s which I will hash and sign in some different platform.
expectation: tx.serialize(false).toString('hex')
currently: RLP.encode(tx.raw.slice(0, 6)).toString('hex')

Once I receive the signature, I should be able to update my transaction easily.
expectation: tx.updateSignature(sig)
currently:

tx.r: sig.signature.slice(0, 32).toString('hex);
tx.s: sig.signature.slice(32, 64).toString('hex);
tx.v: sig.recovery + 27;

Where const sig = secp256k1.sign(txData, privKey)
Else, we can restrict sig to be strictly DER encoded as specified in BIP66

Use case 2 (EIP-155 support)

Create a raw transaction with v=3,r=0 and s=0, get the rlp encoded serialized transaction data by tx.serialize().toString('hex') then hash and sign it in some different platform.

Now, when I update the tx object manually by adding the v,r,s from the received sig with appropriate v as >=37, I always get an invalid signature error.

Unfortunately, verifySignature() method always does const msgHash = this.hash(false) (transaction.ts#L260), which depends on chainId(which I didn't specify while creating the raw tx) to construct the correct hash.

So, it's confusing whether to specify chainId in the raw transaction in the first place or to modify v directly with the correct chainId value.

I think the library is assuming that for every external sign use case, the user will specify the chainId and take the tx.hash(false) rather than modifying the v and taking the tx.serialize() .

@alcuadrado
Copy link
Member

alcuadrado commented May 14, 2019

@rajeshsubhankar could you provide more context?

What motivates use case 1? What do you use the RLP serialization without the signature for?

About use case 2, I can see your frustration. The chainId is only computed in the constructor, not if you modify the v value.

@rajeshsubhankar
Copy link

rajeshsubhankar commented May 16, 2019

@alcuadrado In my use case, multiple parties are involved to verify the transaction by referring RLP serialized data with the actual user request before they hash and sign it.

If the transaction is not yet signed, RLP serialized data with an empty signature doesn't provide much value right?

@alcuadrado
Copy link
Member

alcuadrado commented May 16, 2019

@rajeshsubhankar just to be sure, what would expect from rx.serialize() if the tx hasn't been signed?rlp.encode([nonce, gasPrice, gasLimit, to, value, data])?

@alcuadrado
Copy link
Member

@rajeshsubhankar I think you would find #153 interesting. It simplifies the TX's constructor and the EIP155 logic.

The relevant changes to there are:

  • EIP155 is enabled by default after spurious dragon
  • Serializing an unsigned transaction now returns rlp.encode([nonce, gasPrice, gasLimit, to, value, data, <empty buffer>, <empty buffer>, <empty buffer>])
  • The chain id is always computed from the common object set in the constructor. If you try to use an EIP155 v value that doesn't match it an exception will be thrown.

I think this covers your use case now. You should be able to create an unsigned transaction now, call tx.hash(false), signing somewhere else, validate it with this library after setting v, r, and s.

Note that if you don't want to use EIP155 you should do something like new Transaction(data, {hardfork: "tangerineWhistle"}). I'd recommend sticking with EIP155 though.

@rajeshsubhankar
Copy link

Thanks @alcuadrado for clarifying all my doubts.

@alcuadrado
Copy link
Member

Your welcome @rajeshsubhankar! :)

Just note that the changes in #153 haven't been released yet. A new major version will be out soon, which will include them.

I'm closing this issue now. Feel free to open a new one.

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

No branches or pull requests

3 participants