Introduce a chain agnostic Head interface in the core Transaction Manager #8674
Conversation
… into head_generic
… into head_generic
… into head_generic
core/chains/evm/evm_txm.go
Outdated
| type evmTxm struct { | ||
| httypes.HeadTrackable | ||
| GenericTxManager | ||
| } |
There was a problem hiding this comment.
Where are the comments on lines 18 thru 21?
core/chains/evm/evm_txm.go
Outdated
| func (e evmTxm) OnNewLongestChain(ctx context.Context, head *evmtypes.Head) { | ||
| e.TxManager.OnNewLongestChain(ctx, head) | ||
| } |
There was a problem hiding this comment.
I am not sure why this is necessary but I believe you that it is.
There was a problem hiding this comment.
Same here. I don't know why it doesn't work without this, but if I remove this function, I see these errors:
FAIL github.com/smartcontractkit/chainlink/core/chains/evm [build failed]
github.com/smartcontractkit/chainlink/core/chains/evm [github.com/smartcontractkit/chainlink/core/chains/evm/client.test]
./evm_txm.go:16:31: cannot use &evmTxm{} (value of type *evmTxm) as "github.com/smartcontractkit/chainlink/core/chains/evm/headtracker/types".HeadTrackable value in variable declaration: *evmTxm does not implement "github.com/smartcontractkit/chainlink/core/chains/evm/headtracker/types".HeadTrackable (missing method OnNewLongestChain)
./chain.go:120:28: cannot use txm (variable of type *evmTxm) as "github.com/smartcontractkit/chainlink/core/chains/evm/headtracker/types".HeadTrackable value in argument to headBroadcaster.Subscribe: *evmTxm does not implement "github.com/smartcontractkit/chainlink/core/chains/evm/headtracker/types".HeadTrackable (missing method OnNewLongestChain)
There was a problem hiding this comment.
Huh, weird. Could it have been a pointer issue? I'm surprised to see the receiver here is not a pointer.
There was a problem hiding this comment.
I didn't understand your question.
evmTxm implements both interfaces:
- httypes.HeadTrackable. (requires OnLongestChain(..., *evmtypes.Head))
- txmgr.TxManager (requires OnLongestChain(..., txmgrtypes.Head))
I earlier assumed that in evmTxm, just letting the OnLongestChain method inheriting from TxManager would work, but it doesn't. I have to create above function to pass-through.
There was a problem hiding this comment.
From my understanding of go, this shouldn't be necessary. Can we figure out why it's here?
There was a problem hiding this comment.
This method is not necessary. This is because the type assertion is generic instead of specifying [*evmtypes.Head].
There was a problem hiding this comment.
Er, the concrete type with the same name is not connected to the generic one. This should be removable if they are derived from one another.
There was a problem hiding this comment.
Actually, this whole type is not necessary: #8744
| @@ -159,7 +159,7 @@ func (ht *headTracker) Backfill(ctx context.Context, headWithChain *evmtypes.Hea | |||
| baseHeight = 0 | |||
| } | |||
|
|
|||
| return ht.backfill(ctx, headWithChain.EarliestInChain(), baseHeight) | |||
| return ht.backfill(ctx, headWithChain.EarliestInChain().(*evmtypes.Head), baseHeight) | |||
There was a problem hiding this comment.
Are we going to end up with typecasts everywhere like this now that head is more generic? That seems like a step backwards
There was a problem hiding this comment.
This was due to the fact that EarliestInChain() was already used by evm native code, and also needed for generic Txmgr, so i merged them to a single interface method, that returns interface Head, instead of native head.
Changed it now.
The interface method is different from native method, so avoids having to use different types.
core/chains/evm/evm_txm.go
Outdated
|
|
||
| // evmTxm is an evm wrapper over the generic TxManager interface | ||
| type evmTxm struct { | ||
| httypes.HeadTrackable |
There was a problem hiding this comment.
is this redundant? TxManager already requires txmgrtypes.HeadTrackable[*evmtypes.Head]
There was a problem hiding this comment.
Yes, the whole evmTxm type is actually not neccesssary: #8744
There was a problem hiding this comment.
Thanks a lot Jordan!
… into head_generic
… into head_generic
… into head_generic
|
SonarQube Quality Gate
|
The core Txm shouldn't be importing evmtypes.Head.
Instead it will use a generic Head interface that all chains can provide.
Ticket: smartcontract-it.atlassian.net/browse/BCI-854
PS: This PR was opened after initially trying the following one, which due to use of generics, was far more complicated: #8597