sdk 50 v2 #716
sdk 50 v2 #716
Conversation
catShaark
force-pushed
the
faddat/sdk-50-v2
branch
from
515bb5c
to
2196665
Compare
|
ohhh great point @catShaark -- I'd neglected that step, thank you! |
|
@catShaark -- I was able to stop thi linter issue by explicitly specifying that the linter action should:
|
| @@ -417,46 +417,6 @@ func (tn *ChainNode) FindTxs(ctx context.Context, height uint64) ([]blockdb.Tx, | |||
| } | |||
| txs = append(txs, newTx) | |||
| } | |||
| if len(blockRes.BeginBlockEvents) > 0 { | |||
There was a problem hiding this comment.
Should BeginBlockEvents/EndBlockEvents be replaced with FinalizeBlockEvents instead of just removed?
There was a problem hiding this comment.
I'd need to defer to @catShaark on this. I am not sure, but thanks for the review and bringing this up!
There was a problem hiding this comment.
thanks @misko9, I just add the code u request.
|
quick update -- we've gotten this PR: ready to merge and it.... more or less means that Once that is cut I will get rid of the replace statement in go.mod here. |
|
We are really close. @charleenfei, any chance you know why the CI doesn't like the hermes config?
|
|
path error sort of looks like it cannot find the file, but I am not sure that is the actual issue here. It is also possible that the configuration schema has changed, if you're upgrading hermes from 1.5* to 1.6* or, did you happen to try the master branch for hermes? commit log looks like v1.6.0 doesn't have the sauce needed for v50 |
|
Yea, we solved this on our side. Basically since 1.4.1, Hermes is compiled
with Ubuntu 23 which has a default first User uid:gid of 1001:1001 instead
of 1000:1000, we need to update tjat for the Hermes config.
…On Fri, 01 Sep 2023, 19:01 Jacob Gadikian, ***@***.***> wrote:
path error sort of looks like it cannot find the file, but I am not sure
that is the actual issue here.
It is also possible that the configuration schema has changed, if you're
upgrading hermes from 1.5* to 1.6*
—
Reply to this email directly, view it on GitHub
<#716 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD4ATZZI6JYOWLBNYA6OJDLXYIIHNANCNFSM6AAAAAA3YT4BZE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Module bumped Right before I merge, I will branch off off a |
|
Note: since this is from a fork, I think it will be easiest to update Reason: I can't properly import the right commit of cc: @Reecepbcups |
|
Ahh, yea this requires an odd go.mod quirk to resolve. Will fix after merge, does not seem I can push up to this PR |
| }, | ||
| }, | ||
| }, | ||
| relayerVersion: "colin-event-fix", |
There was a problem hiding this comment.
qq: do you guys have an eta for when the golang relayer going to be updated with the changes in @colin-axner's image?
There was a problem hiding this comment.
just approved colin's pr! should be merged once CI passes
|
We have a test which uses diff --git a/chain/cosmos/chain_node.go b/chain/cosmos/chain_node.go
index 623f37c..02bcb50 100644
--- a/chain/cosmos/chain_node.go
+++ b/chain/cosmos/chain_node.go
@@ -1055,7 +1055,7 @@ func (tn *ChainNode) UnsafeResetAll(ctx context.Context) error {
tn.lock.Lock()
defer tn.lock.Unlock()
- _, _, err := tn.ExecBin(ctx, "unsafe-reset-all")
+ _, _, err := tn.ExecBin(ctx, "comet", "unsafe-reset-all")
return err
}Is there a way we can handle backwards compatibility here? Do we need to? |
|
@colin-axner Yes, we do need to handle backwards compatibility here. After this PR is merged, I can use the UseNewGenesisCommand bool to toggle the cmd format for unsafe-reset-all as well |
This PR updates interchaintest to sdk v50.