-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Relayer: Recover XRPL originated token registration #53
Conversation
* Add contract client integration tests * Add relayer (runners) integration test * Add non-existing accounts error handing for the xrpl_tx_submitter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @wojtek-coreum and @ysv)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dzmitryhil and @wojtek-coreum)
integration-tests/coreum/contract_client_test.go
line 2244 at r1 (raw file):
// try to recover the token with the unexpected current state _, err = contractClient.RecoverXRPLTokenRegistration(ctx, owner, issuer, currency) require.True(t, coreum.IsXRPLTokenNotInactiveError(err), err)
xrpl Token Not Inactive
so is it active or not ?
I guess it is double negation done by mistake
either xrplTokenInactive or xrplTokenNotActive
integration-tests/coreum/contract_client_test.go
line 2338 at r1 (raw file):
require.Equal(t, coreum.TokenStateEnabled, registeredXRPLToken.State) }
nit: this file is getting too big
Shall we split it into multiple by business logic or flows e.g: token registration, token send, contract administration ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @wojtek-coreum and @ysv)
integration-tests/coreum/contract_client_test.go
line 2244 at r1 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
xrpl Token Not Inactive
so is it active or not ?
I guess it is double negation done by mistakeeither xrplTokenInactive or xrplTokenNotActive
It is not Inactive. The only state when you can recover it is Inactive
so when we try to recover with the state which is not Inactive we get that error.
integration-tests/coreum/contract_client_test.go
line 2338 at r1 (raw file):
Previously, ysv (Yaroslav Savchuk) wrote…
nit: this file is getting too big
Shall we split it into multiple by business logic or flows e.g: token registration, token send, contract administration ?
We can think a bit later how to split it, the issue is that it is not so simple :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @wojtek-coreum)
Description
Reviewers checklist:
Authors checklist
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)