Skip to content

Conversation

@0xjocke
Copy link
Contributor

@0xjocke 0xjocke commented Jul 25, 2022

Description

This PR resolves an issue were we pass a nonce missing 0x prefix.

To get the transaction signed on line 404 in trezor/src/index.ts we call Transaction.fromTxData. The Transaction class is coming from @ethereumjs/tx.
When calling fromTxData that package will instantiate a class extending their BaseTransaction. The BaseTransaction constructor will try to convert the nonce to first a buffer and then BigInt see here . To convert the string to a buffer they rely on toBuffer from @ethereumjs/util, this method will throw if passed a string not prefixed with 0x See here

Using an infura provider I consistently got a nonce without 0x. I also fixed some minor type errors.

Checklist

  • The version field in package.json is incremented following semantic versioning
  • The box that allows repo maintainers to update this PR is checked
  • I tested locally to make sure this feature/fix works
  • I have run yarn type-check & yarn build to confirm there are not any associated errors
  • This PR passes the Circle CI checks

@0xjocke
Copy link
Contributor Author

0xjocke commented Jul 25, 2022

Should I do something to trigger the Circle CI checks?

@Adamj1232
Copy link
Contributor

Should I do something to trigger the Circle CI checks?

@bachstatter no worries on the CI checks for now. We are working on adding those back into our new architecture but they do not currently run on branches

Copy link
Contributor

@Adamj1232 Adamj1232 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works well. Thank you!

@Adamj1232 Adamj1232 merged commit c1bcc7d into thirdweb-dev:v2-web3-onboard-develop Jul 26, 2022
This was referenced Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants