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.

Sign up for GitHub

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

Jump to bottom

Convert values coming in from WalletConnect #70

Merged
merged 5 commits into from
Dec 5, 2021
Merged

Convert values coming in from WalletConnect #70

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d6b46e8
Convert incoming WC values from hex or strings to numbers needed for …
jessgusclark Nov 26, 2021
1438790
use correct variable.
jessgusclark Nov 29, 2021
b091686
Requested changes.
jessgusclark Dec 1, 2021
c2fa7eb
Add sample values so it can send.
jessgusclark Dec 1, 2021
d8fd4bd
Rewrite tests to always expect signer and walletAdapters
jessgusclark Dec 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/lib/walletAdapters/WalletConnectAdapter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ describe('Wallet Connect Adapter', () => {
from,
to,
value: '0x3e8',
gas: '25000',
Copy link
Contributor

@ilanolkies ilanolkies Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Should we add two tests? One for the payload with values and another for the defaults that trigger estimations

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a new test and refactored the existing test to use spyon to get the parameters being sent to sendTransaction.

gasPrice: '60000000',
},
]

Expand Down
17 changes: 12 additions & 5 deletions src/lib/walletAdapters/WalletConnectAdapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { TransactionResponse } from '@ethersproject/providers'
import { Signer, constants, utils } from 'ethers'
import { TransactionRequest } from '@ethersproject/abstract-provider'
import { Signer, constants, utils, BigNumber } from 'ethers'
import { RIFWallet } from '../core'
export class WalletConnectAdapter {
private resolvers: IResolver[]
Expand Down Expand Up @@ -41,13 +42,19 @@ class SendTransactionResolver implements IResolver {
async resolve(params: any[]) {
const payload = params.reduce((prev, curr) => ({ ...prev, ...curr }), {})

if (payload.data === '') {
// TODO: assign undefined once the RIFWallet changes are applied
payload.data = constants.HashZero
const formattedPayload: TransactionRequest = {
to: payload.to,
from: payload.from,
nonce: payload.nonce,
data: payload.data || constants.HashZero,
Copy link
Contributor

@ilanolkies ilanolkies Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We need 0x here as no data. HashZero produces 32 bytes

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated this to be 0x but I also made the default for gasPrice and gasLimit undefined. I think this adapter should not set them to zero in case it is reused in a different app. In #72, if the gasLimit/gasPrice are undefined, then it will use the estimates.

value: BigNumber.from(payload.value || 0),
chainId: payload.chainId,
gasLimit: BigNumber.from(payload.gas || 0), // WC's gas to gasLimit
gasPrice: BigNumber.from(payload.gasPrice || 0),
}

return this.signer
.sendTransaction(payload)
.sendTransaction(formattedPayload)
.then((tx: TransactionResponse) => tx.hash)
}
}
Expand Down