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

Feat/v3 viem p #12

Open
wants to merge 22 commits into
base: feat/v3-viem
Choose a base branch
from
Open

Feat/v3 viem p #12

wants to merge 22 commits into from

Conversation

envin3
Copy link

@envin3 envin3 commented Oct 16, 2023

No description provided.

@envin3 envin3 requested a review from oliviera9 October 16, 2023 19:09
const txReceipt: TransactionReceipt = await this._publicClient.waitForTransactionReceipt({
hash: txHash,
confirmations: confirmations ? confirmations : 1,
})
promi.emit('txConfirmed', txReceipt)
return resolve(txReceipt)
} catch (_err) {

Choose a reason for hiding this comment

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

I would leave the txError emitted event

Copy link
Author

Choose a reason for hiding this comment

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

This does not work. Promi here is in a reject status and event emitter do not work for some reason.

const logs = await this._publicClient.getLogs({
fromBlock: _fromBlock,
address: stringUtils.addHexPrefix(_hubAddress),
events: events,
events: pNetworkHubAbi.filter((_fun) => _fun.type === 'event' && _fun.name === _eventName.toString()),

Choose a reason for hiding this comment

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

events is already defined. reuse it

Copy link
Author

Choose a reason for hiding this comment

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

events was created manually starting from pNetworkHubAbi. This means that in case of any change to pNetworkHubAbi events should be manually updated. In this way instead only EVENT_NAMES needs changes.

@@ -137,17 +157,29 @@ export class pTokensSwap {
.on('txConfirmed', (_swapResult: SwapResult) => {
promi.emit('inputTxConfirmed', _swapResult)
})
const outputTx = await this.monitorOutputTransactions(swapResult.operationId)
const interimTxHash = await this.monitorInterimTransactions(swapResult.operationId)

Choose a reason for hiding this comment

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

I would change monitorCrossChainOperations implementation in a way it returns the log instead of tx hash.

In this way you can pass the log directly into the already existing getOperationIdFromLog

Copy link
Author

Choose a reason for hiding this comment

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

This would mean to import Log type on ptoken-swap. It is not manadatory here but SwapResult should return the Log as any otherwise

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