Hermes migration consumer - #5013
Conversation
Signed-off-by: AndreyM <andrey@mysterium.network>
Signed-off-by: AndreyM <andrey@mysterium.network>
Signed-off-by: AndreyM <andrey@mysterium.network>
Signed-off-by: AndreyM <andrey@mysterium.network>
New tequilla API endpoints 1. /:id/migrate-hermes/status: check if there is money on non-active Hermes and payment channel for new Hermes is empty or new channel does not exists 2. /:id/migrate-hermes: starts migration. Opens a new channel for new Hermes. Than withdraw money from old to new channel Signed-off-by: AndreyM <andrey@mysterium.network>
# Conflicts: # tequilapi/endpoints/assets/docs.go # tequilapi/endpoints/identities.go
Guillembonet
left a comment
Guillembonet
left a comment
There was a problem hiding this comment.
overall its looking nice
| if err != nil { | ||
| return fmt.Errorf("error during getting balance: %w", err) | ||
| } | ||
| if crypto.FloatToBigMyst(oldBalanceMigrationMinimumMyst).Cmp(oldBalance) > 0 { |
There was a problem hiding this comment.
I think we should also open channel if there is no balance (and then don't withdraw)
There was a problem hiding this comment.
Agree on this. If there is a channel open on any old hermes (or we're registered would probably be the check to run) and channel open operation is free, we should open a channel, just dont withdraw.
tomasmik
left a comment
tomasmik
left a comment
There was a problem hiding this comment.
Looks pretty good, but it's missing mobile bindings: https://github.com/mysteriumnetwork/node/tree/master/mobile/mysterium
Also about old hermes enumeration. It's a bit tricky imo.. But maybe we should actually enumerate all hermeses for balance checks etc. Another thing we could do is call blockchain to get hermes version and assume something like:
If our current hermes version is v3 we should only check/migrate hermes v2 and ignore all previous hermeses. That makes sense in my head at least.
| if err != nil { | ||
| return fmt.Errorf("error during getting balance: %w", err) | ||
| } | ||
| if crypto.FloatToBigMyst(oldBalanceMigrationMinimumMyst).Cmp(oldBalance) > 0 { |
There was a problem hiding this comment.
Agree on this. If there is a channel open on any old hermes (or we're registered would probably be the check to run) and channel open operation is free, we should open a channel, just dont withdraw.
| if err != nil { | ||
| log.Debug().Msgf("Hermes migration error: open channel failed %s", err.Error()) | ||
| return err | ||
| } else if statusResponse.Status == registry.ChannelStatusFail || statusResponse.Status == registry.ChannelStatusOpen { |
There was a problem hiding this comment.
We print debug log as success even if this status is failed and then continue to do a withdrawal to the new channel that was never opened. Whats the idea behind that?
There was a problem hiding this comment.
Well, idea is Transactor only fails to open a channel if there is someone else, sent this transaction before (to take reward). Correct me if I wrong here, I remember Depot Delivery works in this way
Signed-off-by: AndreyM <andrey@mysterium.network>
Fix mobile bindings During migration check whether identity is registered Open new channel even if lack of balance Signed-off-by: AndreyM <andrey@mysterium.network>
3 participants
No description provided.