Blockchain monitor component #15
Conversation
| @@ -12,4 +12,6 @@ export interface BlockchainClient { | |||
| getBoxesByTokenId<T extends AmountType = string>( | |||
| tokenId: TokenId, | |||
| ): AsyncGenerator<Box<T>[]>; | |||
|
|
|||
| getMempool(): Promise<SignedTransaction[]>; | |||
There was a problem hiding this comment.
@arobsn do you have any feelings on adding the methods needed by the monitoring component to the blockchain client used by plugins?
I was on the fence on if it should be separate so plugin devs are guided into implementing the on* methods instead of calling the APIs manually - decided on it probably not being a big deal for now
There was a problem hiding this comment.
I don't think that exposing such a heavy endpoint is something good, on the other side, bots must be able to check the the mempool.
What do you think about holding the last mempool snapshot as an in-memory store? This way we avoid new calls if plugins need it the the mempool.
There was a problem hiding this comment.
Yep this is a good idea, will do this
src/blockchain/blockchain_monitor.ts
Outdated
| async #monitor() { | ||
| this.logger.debug("Gathering blockchain state"); | ||
|
|
||
| const mempool = await this.#blockchainClient.getMempool(); |
There was a problem hiding this comment.
It would be good to check if the mempool has changed before to download it. The /info/ endpoint returns a property called lastSeenMessageTime which returns the lest time the node received a p2p message. If I'm right, this can be used for such check.
| @@ -12,4 +12,6 @@ export interface BlockchainClient { | |||
| getBoxesByTokenId<T extends AmountType = string>( | |||
| tokenId: TokenId, | |||
| ): AsyncGenerator<Box<T>[]>; | |||
|
|
|||
| getMempool(): Promise<SignedTransaction[]>; | |||
There was a problem hiding this comment.
I don't think that exposing such a heavy endpoint is something good, on the other side, bots must be able to check the the mempool.
What do you think about holding the last mempool snapshot as an in-memory store? This way we avoid new calls if plugins need it the the mempool.
|
Feel free to merge or continue from here ) |
| // if a tx is not included in a block in dropChecks * `n` seconds, | ||
| // then it's probably dropped from the mempool | ||
| if (this.#state.mempoolTxChecks[txId] > this.#maxMempoolTxChecks) { | ||
| // TODO raise mempool dropped, so we still need to keep track of txns not just the id? |
There was a problem hiding this comment.
@arobsn when a tx is dropped from mempool should we include the transaction when raising the event for plugins or do you think txId be enough?
There was a problem hiding this comment.
Good question, I would go with the full transaction to keep hooks homogeneous and avoid more code on plugins side (IMO plugins must be as concise as possible to make auditing process easier).
| // if a tx is not included in a block in dropChecks * `n` seconds, | ||
| // then it's probably dropped from the mempool | ||
| if (this.#state.mempoolTxChecks[txId] > this.#maxMempoolTxChecks) { | ||
| // TODO raise mempool dropped, so we still need to keep track of txns not just the id? |
There was a problem hiding this comment.
Good question, I would go with the full transaction to keep hooks homogeneous and avoid more code on plugins side (IMO plugins must be as concise as possible to make auditing process easier).
closes #13
@arobsn this PR is getting a bit large, I have a issue tracking todos/fixes that I'll do in a follow up PR #16
Pipeline is going to be red but feel free to review & merge anyway