refactor: finish the Solana repository implementation #4422
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
renan061
changed the title
renan061
force-pushed
the
feature/solana-repository
branch
from
d44aba1 to
7418bc6
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #4422 +/- ##
===========================================
+ Coverage 64.75% 65.09% +0.33%
===========================================
Files 469 470 +1
Lines 28522 28510 -12
===========================================
+ Hits 18470 18558 +88
+ Misses 9031 8932 -99
+ Partials 1021 1020 -1
🚀 New features to boost your workflow:
|
| lastSignature = fetchedSignatures[len(fetchedSignatures)-1].Signature | ||
| // GetFirstSignature returns the first signature for the gateway address. | ||
| func (repo *SolanaRepo) GetFirstSignature(ctx context.Context) (solana.Signature, error) { | ||
| opts := rpc.GetSignaturesForAddressOpts{Commitment: rpc.CommitmentFinalized} |
There was a problem hiding this comment.
Should we define a constant for the pageLimit , evenif it is 1000 , in case we need to modify it in the futute
There was a problem hiding this comment.
From what I remember, I deleted the constant we had because it wasn't being used.
If we use it somewhere and I'm mistaken then we should put it back.
|
|
||
| type SolanaRepo struct { | ||
| solanaClient SolanaClient | ||
| // TODO |
| address, | ||
| ) | ||
| } | ||
| // TODO |
There was a problem hiding this comment.
Same : could you add more details for the todo
| func (repo *SolanaRepo) GetAddressLookupTableState(ctx context.Context, | ||
| address solana.PublicKey, | ||
| ) (*alt.AddressLookupTableState, error) { | ||
| client, ok := repo.client.(*rpc.Client) |
There was a problem hiding this comment.
We should not be doing a type assertion against a concrete type.
Chao and Dry client, would still be calling this function?
There was a problem hiding this comment.
I just made this observation on another PR, ALTs is breaking dry and chaos mode because of the way we are handling this RPC client. We should refactor ALT to comply with the interface design.
| // TODO: The Solana repository should be injected as a dependency into this function. We | ||
| // should not have to instantiate the Solana client here. | ||
| blockTime, err := solrepo.New(rpcClient).HealthCheck(ctx) | ||
| // should not have to instantiate the Solana client here, nor provide the gatewayID. |
There was a problem hiding this comment.
Let's create a task for this, as this would need a bigger PR, which affects all other clitns as well
Description
TODO.
Closes #4224.
How Has This Been Tested?