-
Notifications
You must be signed in to change notification settings - Fork 92
feat: [WIP] Hinkal Private Wallet integration #1477
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| import { ExtensionTypes, CurrencyTypes, RequestLogicTypes } from '@requestnetwork/types'; | ||
| import { conversionSupportedNetworks, UnsupportedCurrencyError } from '@requestnetwork/currency'; | ||
| import Erc20FeeProxyPaymentNetwork from './erc20/fee-proxy-contract'; | ||
|
|
||
| const CURRENT_VERSION = '0.0.1'; | ||
|
|
||
| export default class AnyToHinkalWalletErc20ProxyPaymentNetwork extends Erc20FeeProxyPaymentNetwork<ExtensionTypes.PnAnyToHinkalWalletErc20.ICreationParameters> { | ||
| public constructor( | ||
| currencyManager: CurrencyTypes.ICurrencyManager, | ||
| extensionId: ExtensionTypes.PAYMENT_NETWORK_ID = ExtensionTypes.PAYMENT_NETWORK_ID | ||
| .ERC20_HINKAL_WALLET, | ||
| currentVersion: string = CURRENT_VERSION, | ||
| ) { | ||
| super(currencyManager, extensionId, currentVersion); | ||
| } | ||
| /** | ||
| * Creates the extensionsData to create the extension ERC20 fee proxy contract payment detection | ||
| * | ||
| * @param creationParameters extensions parameters to create | ||
| * | ||
| * @returns IExtensionCreationAction the extensionsData to be stored in the request | ||
| */ | ||
| public createCreationAction( | ||
| creationParameters: ExtensionTypes.PnAnyToHinkalWalletErc20.ICreationParameters, | ||
| ): ExtensionTypes.IAction { | ||
| if (!creationParameters.acceptedTokens) { | ||
| throw Error('acceptedTokens is required'); | ||
| } | ||
| if (creationParameters.acceptedTokens.length === 0) { | ||
| throw Error('acceptedTokens cannot be empty'); | ||
| } | ||
| if (creationParameters.acceptedTokens.some((address) => !this.isValidAddress(address))) { | ||
| throw Error('acceptedTokens must contains only valid ethereum addresses'); | ||
| } | ||
| const network = creationParameters.network; | ||
| this.throwIfInvalidNetwork(network); | ||
|
|
||
| for (const address of creationParameters.acceptedTokens) { | ||
| const acceptedCurrency = this.currencyManager.fromAddress(address, network); | ||
| if (!acceptedCurrency) { | ||
| throw new UnsupportedCurrencyError({ | ||
| value: address, | ||
| network, | ||
| }); | ||
| } | ||
| if (!this.currencyManager.supportsConversion(acceptedCurrency, network)) { | ||
| throw Error( | ||
| `acceptedTokens must contain only supported token addresses (ERC20 only). ${address} is not supported for ${network}.`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return super.createCreationAction(creationParameters); | ||
| } | ||
|
|
||
| /** | ||
| * Applies a creation extension action | ||
| * | ||
| * @param extensionAction action to apply | ||
| * @param timestamp action timestamp | ||
| * | ||
| * @returns state of the extension created | ||
| */ | ||
| protected applyCreation( | ||
| extensionAction: ExtensionTypes.IAction, | ||
| timestamp: number, | ||
| ): ExtensionTypes.IState { | ||
| if (!extensionAction.parameters.network || extensionAction.parameters.network.length === 0) { | ||
| throw Error('network is required'); | ||
| } | ||
|
|
||
| if ( | ||
| !extensionAction.parameters.acceptedTokens || | ||
| extensionAction.parameters.acceptedTokens.length === 0 | ||
| ) { | ||
| throw Error('acceptedTokens is required and cannot be empty'); | ||
| } | ||
| if ( | ||
| extensionAction.parameters.acceptedTokens.some( | ||
| (address: string) => !this.isValidAddress(address), | ||
| ) | ||
| ) { | ||
| throw Error('acceptedTokens must contains only valid ethereum addresses'); | ||
| } | ||
|
|
||
| const feePNCreationAction = super.applyCreation(extensionAction, timestamp); | ||
|
|
||
| return { | ||
| ...feePNCreationAction, | ||
| events: [ | ||
| { | ||
| name: 'create', | ||
| parameters: { | ||
| feeAddress: extensionAction.parameters.feeAddress, | ||
| feeAmount: extensionAction.parameters.feeAmount, | ||
| paymentAddress: extensionAction.parameters.paymentAddress, | ||
| refundAddress: extensionAction.parameters.refundAddress, | ||
| salt: extensionAction.parameters.salt, | ||
| network: extensionAction.parameters.network, | ||
| acceptedTokens: extensionAction.parameters.acceptedTokens, | ||
| maxRateTimespan: extensionAction.parameters.maxRateTimespan, | ||
| }, | ||
| timestamp, | ||
| }, | ||
| ], | ||
| values: { | ||
| ...feePNCreationAction.values, | ||
| network: extensionAction.parameters.network, | ||
| acceptedTokens: extensionAction.parameters.acceptedTokens, | ||
| maxRateTimespan: extensionAction.parameters.maxRateTimespan, | ||
| }, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Validate the extension action regarding the currency and network | ||
| * It must throw in case of error | ||
| */ | ||
| protected validate( | ||
| request: RequestLogicTypes.IRequest, | ||
| extensionAction: ExtensionTypes.IAction, | ||
| ): void { | ||
| //TODO: add hinkal network validataion | ||
| const network = | ||
| extensionAction.action === ExtensionTypes.PnFeeReferenceBased.ACTION.CREATE | ||
| ? extensionAction.parameters.network | ||
| : request.extensions[this.extensionId]?.values.network; | ||
| if (!network) { | ||
| return; | ||
| } | ||
|
|
||
| // Nothing can be validated if the network has not been given yet | ||
| if (!network) { | ||
| return; | ||
| } | ||
|
|
||
| if (!conversionSupportedNetworks.includes(network)) { | ||
| throw new Error(`The network (${network}) is not supported for this payment network.`); | ||
| } | ||
|
|
||
| const currency = this.currencyManager.fromStorageCurrency(request.currency); | ||
| if (!currency) { | ||
| throw new UnsupportedCurrencyError(request.currency); | ||
| } | ||
| if (!this.currencyManager.supportsConversion(currency, network)) { | ||
| throw new Error( | ||
| `The currency (${currency.id}, ${currency.hash}) of the request is not supported for this payment network.`, | ||
| ); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,84 @@ | ||||||||||||||
| import { providers } from 'ethers'; | ||||||||||||||
| import { BigNumberish } from 'ethers'; | ||||||||||||||
| import { Signer } from 'ethers'; | ||||||||||||||
| import { ClientTypes, ExtensionTypes } from '@requestnetwork/types'; | ||||||||||||||
| import { | ||||||||||||||
| getAmountToPay, | ||||||||||||||
| getProvider, | ||||||||||||||
| getRequestPaymentValues, | ||||||||||||||
| getSigner, | ||||||||||||||
| validateRequest, | ||||||||||||||
| } from './utils'; | ||||||||||||||
| import { EthersProviderAdapter, Hinkal } from '@hinkal/client'; | ||||||||||||||
| import { ERC20__factory, ERC20Proxy__factory } from '@requestnetwork/smart-contracts/types'; | ||||||||||||||
| import { RelayerTransaction } from '@hinkal/client/dist/types/relay'; | ||||||||||||||
| import { emporiumOp } from '@hinkal/crypto'; | ||||||||||||||
| import { erc20ProxyArtifact } from '@requestnetwork/smart-contracts'; | ||||||||||||||
| import { getPaymentNetworkExtension } from '@requestnetwork/payment-detection'; | ||||||||||||||
| import { EvmChains } from '@requestnetwork/currency'; | ||||||||||||||
|
|
||||||||||||||
| export async function payErc20HinkalWalletProxyRequest( | ||||||||||||||
| request: ClientTypes.IRequestData, | ||||||||||||||
| signerOrProvider: providers.Provider | Signer = getProvider(), | ||||||||||||||
| amount: BigNumberish, | ||||||||||||||
| ): Promise<providers.TransactionResponse> { | ||||||||||||||
| const signer = getSigner(signerOrProvider); | ||||||||||||||
|
|
||||||||||||||
| const { transactionHash } = await constructAndSendTransferOp(request, signer, amount); | ||||||||||||||
|
|
||||||||||||||
| return await signer.provider!.getTransaction(transactionHash); | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using the non-null assertion operator '!'. Using the non-null assertion operator Apply this diff to fix the issue: -return await signer.provider!.getTransaction(transactionHash);
+if (signer.provider) {
+ return await signer.provider.getTransaction(transactionHash);
+} else {
+ throw new Error('Provider is not available on the signer');
+}📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome[error] 20-20: Forbidden non-null assertion. Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator (lint/style/noNonNullAssertion) |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| export async function constructAndSendTransferOp( | ||||||||||||||
| request: ClientTypes.IRequestData, | ||||||||||||||
| signer: Signer, | ||||||||||||||
| amount: BigNumberish, | ||||||||||||||
| ): Promise<RelayerTransaction> { | ||||||||||||||
| validateRequest(request, ExtensionTypes.PAYMENT_NETWORK_ID.ERC20_HINKAL_WALLET); | ||||||||||||||
| const hinkalProviderAdapter = new EthersProviderAdapter(signer, await signer.getChainId()); | ||||||||||||||
|
|
||||||||||||||
| const tokenAddress = request.currencyInfo.value; | ||||||||||||||
| const { network } = request.currencyInfo; | ||||||||||||||
|
|
||||||||||||||
| const tokenInstance = ERC20__factory.connect(tokenAddress, signer.provider!); | ||||||||||||||
|
|
||||||||||||||
| const { paymentReference, paymentAddress } = getRequestPaymentValues(request); | ||||||||||||||
|
|
||||||||||||||
| const amountToPay = getAmountToPay(request, amount); | ||||||||||||||
|
|
||||||||||||||
| // TODO: calculate/get Hinkal fee | ||||||||||||||
|
|
||||||||||||||
| const pn = getPaymentNetworkExtension(request); | ||||||||||||||
| // TODO: add hinkal instance support check also | ||||||||||||||
| EvmChains.assertChainSupported(network!); | ||||||||||||||
|
|
||||||||||||||
| const proxyAddress = erc20ProxyArtifact.getAddress(network, pn?.version); | ||||||||||||||
|
|
||||||||||||||
| const proxyContract = ERC20Proxy__factory.connect(proxyAddress, signer.provider!); | ||||||||||||||
|
|
||||||||||||||
| const hinkal = new Hinkal(); | ||||||||||||||
|
|
||||||||||||||
| await hinkal.initProviderAdapter(undefined, hinkalProviderAdapter); | ||||||||||||||
|
|
||||||||||||||
| await hinkal.initUserKeys(); | ||||||||||||||
|
|
||||||||||||||
| await hinkal.resetMerkle(); | ||||||||||||||
|
|
||||||||||||||
| const approveOp = emporiumOp(tokenInstance, 'approve', [proxyAddress, amountToPay]); | ||||||||||||||
|
|
||||||||||||||
| const transferOp = emporiumOp(proxyContract, 'transferFromWithReference', [ | ||||||||||||||
| tokenAddress, | ||||||||||||||
| paymentAddress, | ||||||||||||||
| amountToPay, | ||||||||||||||
| `0x${paymentReference}`, | ||||||||||||||
| ]); | ||||||||||||||
|
|
||||||||||||||
| return (await hinkal.actionPrivateWallet( | ||||||||||||||
| [tokenAddress], | ||||||||||||||
| [BigInt(amount.toString())], | ||||||||||||||
| [false], | ||||||||||||||
| [approveOp, transferOp], | ||||||||||||||
| undefined, | ||||||||||||||
| false, | ||||||||||||||
| )) as RelayerTransaction; | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,6 +21,7 @@ import { encodeRequestErc20Approval } from './encoder-approval'; | |||||||||
| import { encodeRequestPayment } from './encoder-payment'; | ||||||||||
| import { IPreparedTransaction } from './prepared-transaction'; | ||||||||||
| import { IRequestPaymentOptions } from '../types'; | ||||||||||
| import { payErc20HinkalWalletProxyRequest } from './erc20-hinkal-wallet'; | ||||||||||
| export { INearTransactionCallback } from './utils-near'; | ||||||||||
|
|
||||||||||
| export const noConversionNetworks = [ | ||||||||||
|
|
@@ -71,6 +72,7 @@ export class UnsupportedPaymentChain extends Error { | |||||||||
| * Processes a transaction to pay a Request. | ||||||||||
| * Supported networks: | ||||||||||
| * - ERC20_PROXY_CONTRACT | ||||||||||
| * - ERC20_HINKAL_WALLET | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider enhancing documentation for privacy features While the addition is correct, consider adding a brief note about Hinkal Wallet's privacy features and its specific use case, as it differs from other payment networks in this aspect. Example addition: * Supported networks:
* - ERC20_PROXY_CONTRACT
- * - ERC20_HINKAL_WALLET
+ * - ERC20_HINKAL_WALLET (Privacy-focused wallet using ZK-proofs for confidential transactions)
* - ETH_INPUT_DATA📝 Committable suggestion
Suggested change
|
||||||||||
| * - ETH_INPUT_DATA | ||||||||||
| * - ERC20_FEE_PROXY_CONTRACT | ||||||||||
| * - ANY_TO_ERC20_PROXY | ||||||||||
|
|
@@ -99,6 +101,8 @@ export async function payRequest( | |||||||||
| case ExtensionTypes.PAYMENT_NETWORK_ID.ERC20_FEE_PROXY_CONTRACT: | ||||||||||
| case ExtensionTypes.PAYMENT_NETWORK_ID.ERC20_TRANSFERABLE_RECEIVABLE: | ||||||||||
| return payErc20Request(request, signer, amount, undefined, overrides); | ||||||||||
| case ExtensionTypes.PAYMENT_NETWORK_ID.ERC20_HINKAL_WALLET: | ||||||||||
| return payErc20HinkalWalletProxyRequest(request, signer, amount!); | ||||||||||
| case ExtensionTypes.PAYMENT_NETWORK_ID.ERC777_STREAM: | ||||||||||
| return payErc777StreamRequest(request, signer); | ||||||||||
| case ExtensionTypes.PAYMENT_NETWORK_ID.ANY_TO_ERC20_PROXY: { | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -207,3 +207,6 @@ export type CurrencyPairs = Record<string, Record<string, number>>; | |
| * Network => currencyFrom => currencyTo => cost | ||
| */ | ||
| export type AggregatorsMap<T extends ChainName = ChainName> = Partial<Record<T, CurrencyPairs>>; | ||
|
|
||
| //For now only optimism is supported to work with Hinkal Wallet | ||
| export type HinkalSupportedNetworks = 'optimism'; | ||
|
Comment on lines
+210
to
+212
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider relocating the HinkalSupportedNetworks type. For better code organization, consider moving this type definition near other network-related types at the beginning of the file (around line 4 where |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ import * as PnMeta from './extensions/pn-meta'; | |
| import * as PnAnyToAnyConversion from './extensions/pn-any-to-any-conversion-types'; | ||
| import * as Identity from './identity-types'; | ||
| import * as RequestLogic from './request-logic-types'; | ||
| import * as PnAnyToHinkalWalletErc20 from './extensions/pn-any-hinkal-wallet-based-types'; | ||
|
|
||
| export { | ||
| ContentData, | ||
|
|
@@ -22,6 +23,7 @@ export { | |
| PnAnyToEth, | ||
| PnAnyToAnyConversion, | ||
| PnMeta, | ||
| PnAnyToHinkalWalletErc20, | ||
| }; | ||
|
|
||
| /** Extension interface is extended by the extensions implementation */ | ||
|
|
@@ -89,6 +91,7 @@ export enum PAYMENT_NETWORK_ID { | |
| BITCOIN_ADDRESS_BASED = 'pn-bitcoin-address-based', | ||
| TESTNET_BITCOIN_ADDRESS_BASED = 'pn-testnet-bitcoin-address-based', | ||
| ERC20_ADDRESS_BASED = 'pn-erc20-address-based', | ||
| ERC20_HINKAL_WALLET = 'pn-erc20-hinkal-wallet', | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider adding type constraints for security-critical operations. Given that this is part of a private wallet integration using zero-knowledge proofs, consider:
|
||
| ERC20_PROXY_CONTRACT = 'pn-erc20-proxy-contract', | ||
| ERC20_FEE_PROXY_CONTRACT = 'pn-erc20-fee-proxy-contract', | ||
| ERC777_STREAM = 'pn-erc777-stream', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import * as PnAnyToAnyConversion from './pn-any-to-any-conversion-types'; | ||
| import { HinkalSupportedNetworks } from '../currency-types'; | ||
|
Comment on lines
+1
to
+2
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification HinkalSupportedNetworks type needs to be expanded The current implementation only supports 'optimism' network, but the PR objectives mention support for multiple networks (Polygon, Arbitrum, Ethereum). Update the export type HinkalSupportedNetworks = 'optimism' | 'polygon' | 'arbitrum' | 'ethereum';🔗 Analysis chainVerify completeness of supported networks The PR objectives mention support for multiple networks (Polygon, Arbitrum, Ethereum, etc.), but Let me try another search to find the definition of 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check the definition of HinkalSupportedNetworks
rg -A 5 "type HinkalSupportedNetworks" "packages/types/src/currency-types.ts"
Length of output: 28780 Script: #!/bin/bash
# Search for HinkalSupportedNetworks definition
rg -A 5 "type HinkalSupportedNetworks|enum HinkalSupportedNetworks" "packages/types/src/currency-types.ts"
Length of output: 156 |
||
|
|
||
| /** Any to Hinkal Wallet reference-based payment network extension interface */ | ||
| export type IAnyToHinkalWalletErc20 = | ||
| PnAnyToAnyConversion.IConversionReferenceBased<ICreationParameters>; | ||
|
|
||
| /** Parameters for the creation action */ | ||
| export interface ICreationParameters extends PnAnyToAnyConversion.ICreationParameters { | ||
| network: HinkalSupportedNetworks; | ||
| acceptedTokens: string[]; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle potential issues with 'getSigner' utility.
Ensure that the
getSignerutility function properly handles all possible inputs forsignerOrProvider. Passing a provider when a signer is required could lead to runtime errors.Consider adding validation or error handling within
getSignerto confirm that a valid signer is returned.