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

isSigned returning true for unsigned transactions #1041

Closed
alexfmpe opened this issue Jan 12, 2021 · 3 comments
Closed

isSigned returning true for unsigned transactions #1041

alexfmpe opened this issue Jan 12, 2021 · 3 comments

Comments

@alexfmpe
Copy link
Contributor

Creating a transaction with fromTxData without the signature fields leads to them being initialized

v: <BN: 0>,
r: <BN: 0>,
s: <BN: 0>,

which makes isSigned return true
https://github.com/ethereumjs/ethereumjs-vm/blob/3588bd8dbc744eec1b58c4014cef3838d0a539aa/packages/tx/src/transaction.ts#L359

> !!0
false
> !!(new BN(0))
true
@ryanio
Copy link
Contributor

ryanio commented Jan 12, 2021

ah good catch, they should indeed not be initialized if missing, so these lines in the constructor:

    this.v = new BN(toBuffer(v))
    this.r = new BN(toBuffer(r))
    this.s = new BN(toBuffer(s))

should change to:

    this.v = v ? new BN(toBuffer(v)) : undefined
    this.r = r ? new BN(toBuffer(r)) : undefined
    this.s = s ? new BN(toBuffer(s)) : undefined

@holgerd77
Copy link
Member

Oh, that's an important one, I'll add a respective label here and we should release relatively soon once we have merged the respective PR from @ryanio (thanks 😄 ). Think we are generally at a point where it makes sense to target a new round of releases after the initial v5 releases.

@holgerd77
Copy link
Member

Closed by #1042

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

No branches or pull requests

3 participants