chore: remove `cosmos/relayer` dependency #429

Lazar955 · 2025-01-21T11:34:42Z

Migrates the minimal amount of code needed from the relayer repo
Removes dependency for cosmos/relayer
Removes dependency for strangelove-ventures/cometbft-client

~~Todo:~~

As this is modified code from the relayer, what license if any should we include in the header of those files? I've also modified them (deleted a bunch of stuff, and added more code quality improvements)
~~Is feegrant.go needed, feels like it can be removed and other parts related to it~~
~~Future PRs can remove references to strangelove cometbft client, tho we do have to satisfy the CometRPC interface, ABCIClient interface etc. (not sure if it's worth the extra effort)~~
~~Decide on the name of the relayer package, the clients would currently use relayerclient.Whatever~~

Closes

RafilxTenfen · 2025-01-21T15:20:50Z

are we going to have our own relayer?

KonradStaniec · 2025-01-21T17:15:59Z

are we going to have our own relayer?

Not really.

Main goal of this task is to make our programs to not depend on libraries which are no longer maintained (in this case github.com/strangelove-ventures/cometbft-client ) and this requires as to ditch dependency on go-relayer

KonradStaniec · 2025-01-21T17:42:19Z

Decide on the name of the relayer package, the clients would currently use relayerclient.Whatever

tbh I would name it babylonclient or cosmosclient

As this is modified code from the relayer, what license if any should we include in the header of those files? I've also modified them (deleted a bunch of stuff, and added more code quality improvements)

Something like that - https://law.stackexchange.com/questions/59999/can-i-copy-pieces-of-apache-licensed-source-code-if-i-attribute ? (though I am not a license expert)

Future PRs can remove references to strangelove cometbft client, tho we do have to satisfy the CometRPC interface, ABCIClient interface etc. (not sure if it's worth the extra effort)

I think one of the goals of this is to, remove dependency on strangelove cometbft client , so maybe lets:

create feature branch with feat/ prefix for the whole feature
make this pr to the feature branch
then create next pr removing dependency on strangelove cometbft clien
then create pr from feature branch to main

Or maybe remove strangelove cometbft client directly in this pr. wdyt ?

Lazar955 · 2025-01-21T19:36:07Z

@KonradStaniec

Or maybe remove strangelove cometbft client directly in this pr. wdyt ?

Removed it in this PR, it wasn't a big effort, commit

RafilxTenfen · 2025-01-22T01:03:16Z

The package relayerclient could be in a separate repository in my opinion

Lazar955 · 2025-01-22T12:15:48Z

The package relayerclient could be in a separate repository in my opinion
@RafilxTenfen
For now, I would keep it within the Babylon repo. The only "issue" I personally see is that updating client code would mean that we have to release a new version of Babylon, I don't see this as something that should happen often (as up to this point it wasn't the case), as the only real dependencies of that new package are cometbft and cosmos SDK. If we start updating the client often and as a consequence we have to release new version of Babylon, I'm down for splitting this into it's own repo.

RafilxTenfen · 2025-01-22T12:32:46Z

The package relayerclient could be in a separate repository in my opinion
@RafilxTenfen
For now, I would keep it within the Babylon repo. The only "issue" I personally see is that updating client code would mean that we have to release a new version of Babylon, I don't see this as something that should happen often (as up to this point it wasn't the case), as the only real dependencies of that new package are cometbft and cosmos SDK. If we start updating the client often and as a consequence we have to release new version of Babylon, I'm down for splitting this into it's own repo.

Yeah, but also like we are constantly getting babylon node code checked and this is basically not a part of it, just a consumer
Anyway, if we intend to maintain this code in the Babylon repo, the PR looks good to me ^^

RafilxTenfen

LGTM

KonradStaniec

LGTM!

One question, did you test this branch with some service (like use this version in staker and run e2e tests) to see whether change is really not breaking ?

Also, lets merge this one after - #402

Lazar955 · 2025-01-23T09:26:33Z

LGTM!

One question, did you test this branch with some service (like use this version in staker and run e2e tests) to see whether change is really not breaking ?

Also, lets merge this one after - #402

I've tested it with Vigilante e2e tests

RafilxTenfen · 2025-01-23T11:23:11Z

client/babylonclient/codec.go

+	"github.com/cosmos/cosmos-sdk/codec/types"
+)
+
+type Codec struct {


This seems the same as our EncodingConfig

Can we use that?

sure, nice spot

RafilxTenfen · 2025-01-23T11:24:38Z

client/client/client.go

 	"go.uber.org/zap"
 )

 type Client struct {
 	*query.QueryClient

-	provider *cosmos.CosmosProvider
+	provider *babylonclient.CosmosProvider


Can we make this public?

Suggested change

provider *babylonclient.CosmosProvider

Provider *babylonclient.CosmosProvider

Yes we can add a getter func

RafilxTenfen · 2025-01-23T11:26:07Z

client/babylonclient/account.go

+
+// GetAccountNumberSequence returns sequence and account number for the given address.
+// It returns an error if the account couldn't be retrieved from the state.
+func (cc *CosmosProvider) GetAccountNumberSequence(clientCtx client.Context, addr sdk.AccAddress) (uint64, uint64, error) {


can we make a function that increases the account sequence also in the WalletState

RafilxTenfen · 2025-01-23T11:35:30Z

client/babylonclient/tx.go

+// This method will return once the transaction has entered the mempool.
+// In an async goroutine, will wait for the tx to be included in the block unless asyncCtx exits.
+// If there is no error broadcasting, the asyncCallback will be called with success/failure of the wait for block inclusion.
+func (cc *CosmosProvider) SendMessagesToMempool(


Can we add another option here to specify the sequence in the msg and if it is specified the PrepareFactory would not load from the account sequence

Hm it's going to be a bit much returning and passing account sequence to build messages, maybe we a separate SendMessagesToMempool?

could be as well ;D

Maybe all this is a bit out of the scope of this PR, comparing what's needed in cov emulator PR?

it could be done in another PR, but just adding the account sequence would help to reduce a lot the code copy in covd

RafilxTenfen · 2025-01-23T11:37:13Z

Added a new comments here #429 due to comment in babylonlabs-io/covenant-emulator#98 (comment)

Lazar955 added 11 commits January 20, 2025 15:38

moving relayer code

56df3aa

moves tests and rm excess

ed114fb

rm excess

5199019

cleanup

9f5fe6c

cleanup

7433cfd

cleanup more

28b7931

cleanup more

095b25e

moar cleanup

d1985b8

lint fixes

9aca0d2

handling nil derefs

44182cd

changelog

7313371

Lazar955 requested a review from KonradStaniec January 21, 2025 12:01

Lazar955 added 2 commits January 21, 2025 14:00

rm fee grant

9bab3f4

lint fixes

3742371

Lazar955 marked this pull request as ready for review January 21, 2025 14:45

Lazar955 requested review from gitferry and RafilxTenfen January 21, 2025 14:46

update gomod

c9ee3bc

remove strangelove dep, use cmt client

0ba6efc

Lazar955 added 2 commits January 21, 2025 20:44

remove fee grant leftovers

561f681

add notice headers

c95b01e

rename package

bc55285

Lazar955 mentioned this pull request Jan 22, 2025

chore: send one msg per tx if batch of msgs fails babylonlabs-io/covenant-emulator#98

Open

cleanup more

fc55f54

Lazar955 requested a review from SebastianElvis January 22, 2025 12:15

RafilxTenfen approved these changes Jan 22, 2025

View reviewed changes

KonradStaniec approved these changes Jan 23, 2025

View reviewed changes

RafilxTenfen reviewed Jan 23, 2025

View reviewed changes

RafilxTenfen self-requested a review January 23, 2025 11:37

Lazar955 added 3 commits January 23, 2025 13:05

pr comments

0a1e13e

revert logger

9c8e50f

gomod merge

f902757

Lazar955 merged commit 2b8c528 into main Jan 23, 2025
24 checks passed

Lazar955 deleted the lazar/remove-relayer-dep branch January 23, 2025 15:37

Lazar955 added a commit that referenced this pull request Jan 24, 2025

chore: remove cosmos/relayer dependency (#429) [backport] (#440)

42e1e6d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: remove `cosmos/relayer` dependency #429

chore: remove `cosmos/relayer` dependency #429

Lazar955 commented Jan 21, 2025 •

edited

Loading

RafilxTenfen commented Jan 21, 2025

KonradStaniec commented Jan 21, 2025

KonradStaniec commented Jan 21, 2025 •

edited

Loading

Lazar955 commented Jan 21, 2025

RafilxTenfen commented Jan 22, 2025

Lazar955 commented Jan 22, 2025

RafilxTenfen commented Jan 22, 2025

RafilxTenfen left a comment

KonradStaniec left a comment

Lazar955 commented Jan 23, 2025

RafilxTenfen Jan 23, 2025

Lazar955 Jan 23, 2025

RafilxTenfen Jan 23, 2025

Lazar955 Jan 23, 2025

RafilxTenfen Jan 23, 2025

RafilxTenfen Jan 23, 2025

Lazar955 Jan 23, 2025

RafilxTenfen Jan 23, 2025

Lazar955 Jan 23, 2025

RafilxTenfen Jan 23, 2025

RafilxTenfen commented Jan 23, 2025

	provider *babylonclient.CosmosProvider
	Provider *babylonclient.CosmosProvider

chore: remove cosmos/relayer dependency #429

chore: remove cosmos/relayer dependency #429

Conversation

Lazar955 commented Jan 21, 2025 • edited Loading

RafilxTenfen commented Jan 21, 2025

KonradStaniec commented Jan 21, 2025

KonradStaniec commented Jan 21, 2025 • edited Loading

Lazar955 commented Jan 21, 2025

RafilxTenfen commented Jan 22, 2025

Lazar955 commented Jan 22, 2025

RafilxTenfen commented Jan 22, 2025

RafilxTenfen left a comment

Choose a reason for hiding this comment

KonradStaniec left a comment

Choose a reason for hiding this comment

Lazar955 commented Jan 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RafilxTenfen commented Jan 23, 2025

chore: remove `cosmos/relayer` dependency #429

chore: remove `cosmos/relayer` dependency #429

Lazar955 commented Jan 21, 2025 •

edited

Loading

KonradStaniec commented Jan 21, 2025 •

edited

Loading