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

fixes #2531 value type on function overload #2561

Merged
merged 14 commits into from
Aug 6, 2024
Merged
5 changes: 5 additions & 0 deletions .changeset/new-hotels-sell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"viem": patch
---

fixes #2560: infer the correct `value` type on function overload by matching correct function using function args
tmm marked this conversation as resolved.
Show resolved Hide resolved
22 changes: 15 additions & 7 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions src/actions/public/estimateContractGas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export type EstimateContractGasParameters<
UnionOmit<EstimateGasParameters<chain>, 'data' | 'to' | 'value'> &
GetValue<
abi,
'nonpayable' | 'payable',
functionName,
EstimateGasParameters<chain> extends EstimateGasParameters
? EstimateGasParameters<chain>['value']
Expand Down
1 change: 1 addition & 0 deletions src/actions/public/simulateContract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export type SimulateContractParameters<
> &
GetValue<
abi,
'nonpayable' | 'payable',
functionName,
CallParameters<derivedChain> extends CallParameters
? CallParameters<derivedChain>['value']
Expand Down
12 changes: 12 additions & 0 deletions src/actions/wallet/writeContract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,18 @@ test('overloaded function', async () => {
).toBeDefined()
})

test('payable overloaded function', async () => {
expect(
await writeContract(client, {
...wagmiContractConfig,
account: accounts[0].address,
functionName: 'mint',
args: [69420n, 1n],
value: 1n,
}),
).toBeDefined()
})

test('w/ simulateContract', async () => {
const { request } = await simulateContract(client, {
...wagmiContractConfig,
Expand Down
4 changes: 3 additions & 1 deletion src/actions/wallet/writeContract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ export type WriteContractParameters<
GetAccountParameter<account> &
GetValue<
abi,
'nonpayable' | 'payable',
functionName,
FormattedTransactionRequest<derivedChain>['value']
FormattedTransactionRequest<derivedChain>['value'],
args
> & {
/** Data to append to the end of the calldata. Useful for adding a ["domain" tag](https://opensea.notion.site/opensea/Seaport-Order-Attributions-ec2d69bf455041a5baa490941aad307f). */
dataSuffix?: Hex | undefined
Expand Down
2 changes: 1 addition & 1 deletion src/experimental/eip5792/actions/writeContracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,4 +151,4 @@ export type WriteContractFunctionParameters<
| (functionName extends allFunctionNames ? functionName : never) // infer value
args?: (abi extends Abi ? UnionWiden<args> : never) | allArgs | undefined
} & (readonly [] extends allArgs ? {} : { args: Widen<args> }) &
GetValue<abi, functionName>
GetValue<abi, mutability, functionName>
1 change: 1 addition & 0 deletions src/op-stack/actions/estimateContractL1Fee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export type EstimateContractL1FeeParameters<
> &
GetValue<
abi,
'nonpayable' | 'payable',
functionName,
EstimateL1FeeParameters<
chain,
Expand Down
1 change: 1 addition & 0 deletions src/op-stack/actions/estimateContractL1Gas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export type EstimateContractL1GasParameters<
> &
GetValue<
abi,
'nonpayable' | 'payable',
functionName,
EstimateL1GasParameters<
chain,
Expand Down
1 change: 1 addition & 0 deletions src/op-stack/actions/estimateContractTotalFee.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export type EstimateContractTotalFeeParameters<
> &
GetValue<
abi,
'nonpayable' | 'payable',
functionName,
EstimateTotalFeeParameters<
chain,
Expand Down
1 change: 1 addition & 0 deletions src/op-stack/actions/estimateContractTotalGas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export type EstimateContractTotalGasParameters<
> &
GetValue<
abi,
'nonpayable' | 'payable',
functionName,
EstimateTotalGasParameters<
chain,
Expand Down
16 changes: 11 additions & 5 deletions src/types/contract.test-d.ts
Copy link
Member

Choose a reason for hiding this comment

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

Let's revert this change.

Original file line number Diff line number Diff line change
Expand Up @@ -221,19 +221,23 @@ test('GetEventArgs', () => {

test('GetValue', () => {
// payable
type Result = GetValue<typeof seaportAbi, 'fulfillAdvancedOrder'>
type Result = GetValue<typeof seaportAbi, 'payable', 'fulfillAdvancedOrder'>
expectTypeOf<Result>().toEqualTypeOf<{ value?: bigint }>()

// other
expectTypeOf<GetValue<typeof seaportAbi, 'getOrderStatus'>>().toEqualTypeOf<{
expectTypeOf<
GetValue<typeof seaportAbi, 'view', 'getOrderStatus'>
>().toEqualTypeOf<{
value?: never
}>()
expectTypeOf<GetValue<typeof seaportAbi, 'cancel'>>().toEqualTypeOf<{
expectTypeOf<
GetValue<typeof seaportAbi, 'nonpayable', 'cancel'>
>().toEqualTypeOf<{
value?: never
}>()

// unknown abi
expectTypeOf<GetValue<Abi, 'foo'>>().toEqualTypeOf<{
expectTypeOf<GetValue<Abi, 'nonpayable' | 'payable', 'foo'>>().toEqualTypeOf<{
value?: bigint | undefined
}>()
const abi = [
Expand All @@ -244,7 +248,9 @@ test('GetValue', () => {
outputs: [{ type: 'uint256' }],
},
]
expectTypeOf<GetValue<typeof abi, 'foo'>>().toEqualTypeOf<{
expectTypeOf<
GetValue<typeof abi, 'nonpayable' | 'payable', 'foo'>
>().toEqualTypeOf<{
value?: bigint | undefined
}>()
})
Expand Down
13 changes: 11 additions & 2 deletions src/types/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,10 +303,19 @@ export type EventDefinition = `${string}(${string})`

export type GetValue<
abi extends Abi | readonly unknown[],
functionName extends string,
mutability extends AbiStateMutability = AbiStateMutability,
Copy link
Contributor Author

@Yuripetusko Yuripetusko Jul 30, 2024

Choose a reason for hiding this comment

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

Ideally mutability should be inferred also, but it's hard to do because of the order of generics arguments (args would be needed to infer mutability on overloads, and mutability is needed to infer args). Also it sucks that it's a second argument of this Generic (because subsequent arguments require it), as it makes it hard to skip it if you don't want to pass it. But I couldn't think of a better way right now, or it would require a bigger refactor

Copy link
Member

Choose a reason for hiding this comment

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

Since GetValue is a public API (exported in index.ts), let's pull this logic directly into simulateContract.ts for now.

functionName extends ContractFunctionName<
abi,
mutability
> = ContractFunctionName<abi, mutability>,
valueType = TransactionRequest['value'],
args extends ContractFunctionArgs<
abi,
mutability,
functionName
> = ContractFunctionArgs<abi, mutability, functionName>,
abiFunction extends AbiFunction = abi extends Abi
? ExtractAbiFunction<abi, functionName>
? ExtractAbiFunctionForArgs<abi, mutability, functionName, args>
: AbiFunction,
_Narrowable extends boolean = IsNarrowable<abi, Abi>,
> = _Narrowable extends true
Expand Down
2 changes: 1 addition & 1 deletion src/utils/abi/decodeFunctionData.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test('default', () => {
functionName: 'isApprovedForAll'
}
| {
args: readonly [] | readonly [bigint]
args: readonly [] | readonly [bigint] | readonly [bigint, bigint]
functionName: 'mint'
}
| {
Expand Down
10 changes: 10 additions & 0 deletions test/src/abis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1363,6 +1363,16 @@ export const wagmiContractConfig = {
stateMutability: 'nonpayable',
type: 'function',
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see that this config has a contract address, do I need to add this function to the actual contract? or it's fine to just have this dummy/placeholder abi for a contract that is not actually deployed?

Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this addition to the ABI.

inputs: [
{ name: 'tokenId', type: 'uint256' },
{ name: 'quantity', type: 'uint256' },
],
name: 'mint',
outputs: [],
stateMutability: 'payable',
type: 'function',
},
{
inputs: [],
name: 'name',
Expand Down
Loading