fix: create account estimate fees #22
Conversation
| convertToGenericAddress<ChainType.EVM>( | ||
| data.addressToInvite, | ||
| ChainType.EVM, | ||
| ), |
There was a problem hiding this comment.
The address to invite should already by generic. You are inviting an address on folksChainIdToInvite which may not neccessarily be EVM
| export function getAdapterAddress( | ||
| folksChainId: FolksChainId, | ||
| network: NetworkType, | ||
| adapterType: AdapterType, | ||
| ) { | ||
| if (isHubChain(folksChainId, network)) | ||
| return getHubChainAdapterAddress(network, adapterType); | ||
| return getSpokeChainAdapterAddress(folksChainId, network, adapterType); | ||
| } |
There was a problem hiding this comment.
When calling getAdapterAddress with Avalanche, you may still be calling from the context of being a spoke in which case you should be calling getSpokeChainAdapterAddress. Therefore it may be best to delete this function entirely and then call getHubChainAdapterAddress / getSpokeChainAdapterAddress directly depending on the context.
| export function buildEvmMessageToSend( | ||
| messageToSendBuilderParams: MessageToSendBuilderParams, | ||
| messageToSendBuilderParams: MessageBuilderParams, | ||
| feeParams: OptionalMessageParams, |
There was a problem hiding this comment.
You are naming this feeParams but technically it could be any of the message params which is being overrided
There was a problem hiding this comment.
the type is: type OptionalMessageParams = Partial<MessageParams>; so related to the fees plus the adapters, I think calling it params or messageParams can be misleading
There was a problem hiding this comment.
Someone may mistakenly pass in feeParams which overrides the adapters. I think the type of feeParams should be changed to only include the params related to fees. Alternatively you ignore the non-fee params of feeParams by not using the spread operator ...feeParams
There was a problem hiding this comment.
Created the FeeParams type instead 53d21c1
| buildMessagePayload( | ||
| messageBuilderParams.action, | ||
| messageBuilderParams.accountId, | ||
| getRandomGenericAddress(), |
There was a problem hiding this comment.
I don't think a random address works here in place of the user address. The transaction will likely revert when trying to estimate gas because the address is not registered to the account id.
You probably want to include the user address in MessageBuilderParams.
src/common/utils/chain.ts
Outdated
| export function getSignerGenericAddress( | ||
| folksChainSigner: FolksChainSigner, | ||
| ): GenericAddress { | ||
| const chainType = folksChainSigner.chainType; | ||
| switch (chainType) { | ||
| case ChainType.EVM: | ||
| return getEvmSignerAddress(folksChainSigner.signer); | ||
| default: | ||
| return exhaustiveCheck(chainType); | ||
| } | ||
| } |
There was a problem hiding this comment.
This isn't returning the generic address?
There was a problem hiding this comment.
yes thanks 86989f5
palace22
changed the title
No description provided.