fix: refactored tx submit checker initialisation #44
Conversation
# Conflicts: # cmd/neutron_query_relayer/main.go # internal/app/app.go
| func (tc *TxSubmitChecker) Run(ctx context.Context) error { | ||
| // we don't want to start jobs before we read all pending txs from database, | ||
| // hence we block on this read operation right in the beginning | ||
| func (tc *TxSubmitChecker) Run(ctx context.Context, submittedTxsTasksQueue <-chan relay.PendingSubmittedTxInfo) error { |
There was a problem hiding this comment.
I'm not sure this is good idea to move submittedTxsTasksQueue into Run(), cause we kinda give idea that you can use this function with multiple queues on the same object. But you're not, cause it is tied to concrete storage that is in TxSubmitChecker struct
There was a problem hiding this comment.
not sure what your point here is; you can, for example, run multiple instances of TxSubmitChecker in separate goroutines to enable parallel processing of pending txs
cmd/neutron_query_relayer/main.go
Outdated
| logger.Fatal("Failed to create NewDefaultStorage", zap.Error(err)) | ||
| } | ||
| defer func(storage relay.Storage) { | ||
| if cfg.AllowTxQueries { |
There was a problem hiding this comment.
now I reailse that this suggestion was a piece of shit #44 (comment): storage is also used by KV processor, so we are not able to have a nil storage on AllowTxQueries==false and therefore should close storage regardless of tx query allowance
There was a problem hiding this comment.
So, does that mean we always need storage to be correctly initialised in any workflow? Why wouldn't we just always initialise it then? That will allow us to remove dummy storage implementation completely, and also remove some useless logic around it.
There was a problem hiding this comment.
exactly, that's the idea
There was a problem hiding this comment.
okay, i'll revert that
There was a problem hiding this comment.
Dummy storage implementation is still present though, don't we need to remove it since we don't use it?
internal/app/app.go
Outdated
| func NewDefaultTxSubmitChecker(cfg config.NeutronQueryRelayerConfig, logRegistry *nlogger.Registry, | ||
| storage relay.Storage) (relay.TxSubmitChecker, error) { | ||
| neutronClient, err := raw.NewRPCClient(cfg.NeutronChain.RPCAddr, cfg.NeutronChain.Timeout, | ||
| logRegistry.Get(TargetChainRPCClientContext)) |
There was a problem hiding this comment.
wrongly passed context: should be NeutronChainRPCClientContext
internal/txprocessor/txprocessor.go
Outdated
| QueryID: queryID, | ||
| SubmittedTxHash: hash, | ||
| NeutronHash: neutronTxHash, | ||
| SubmitTime: time.Now(), |
There was a problem hiding this comment.
this submission time 1) is not correct; 2) is not used by the submit checker. wyt we can do with it? I'd remove it cuz it misleads
There was a problem hiding this comment.
yup, i don't see any usages anywhere in the code. removed this field
# Conflicts: # .env.example # .env.example.dev # cmd/neutron_query_relayer/main.go # internal/app/app.go # internal/relay/relayer.go # internal/txprocessor/txprocessor.go
zavgorodnii
dismissed stale reviews from pr0n00gler and sotnikov-s
via
6021acb
This PR refactors the submit checker:
HOW TO TEST THIS CODE?
Tun the tx and kv integration tests using the audit fix branches for neutron and neutron-contracts, this branch for the relayer, and the main branch for the integration tests.