Skip to content

Smart settle - #5113

Merged
Guillembonet merged 1 commit into
masterfrom
smart-auto-settle
May 16, 2022
Merged

Smart settle#5113
Guillembonet merged 1 commit into
masterfrom
smart-auto-settle

Conversation

@Guillembonet

@Guillembonet Guillembonet commented May 12, 2022

Copy link
Copy Markdown
Contributor

Closes: #5067

Current problems:

  • We settle at 5 MYST always and maybe fees are huge when we do, it's a matter of luck.
  • We call transactor /fee endpoint a lot. In the last 3 minutes there are 1400 calls yet coming only from 550 different nodes.

Proposed solutions:

  • Automatic settlements will be smarter now and work as follows:
    • We have 3 values: minimum to settle(5), max fee percentage(5%), and max unsettled(20)
    • If unsettled is below minimum we don't settle
    • If unsettled is above the maximum we always auto-settle
    • If it's somewhere in between we automatically settle if the transaction fees are less than the percentage (i.e. if we have 6 MYST unsettled we will settle as long as the fees are less than 0.3 MYST)
  • Transactor fees are now cached for 3 minutes 30 seconds (by default) instead of calling every time. Keep in mind that the smart settlement check requires a call there on every hermes promise received (only if we have more unsettled than 5), so with both changes, we might still call transactor /fee endpoint quite a lot but I think it should be less. We can discuss whether we should increase this cache amount a bit more (more than 10 mins will for sure be bad).

Signed-off-by: Guillem Bonet guillem@mysterium.network

@codecov-commenter

codecov-commenter commented May 16, 2022

Copy link
Copy Markdown

Codecov Report

Merging #5113 (2df76f7) into master (cdaa020) will decrease coverage by 0.08%.
The diff coverage is 29.11%.

@@            Coverage Diff             @@
##           master    #5113      +/-   ##
==========================================
- Coverage   39.11%   39.03%   -0.09%     
==========================================
  Files         359      359              
  Lines       19155    19208      +53     
==========================================
+ Hits         7493     7497       +4     
- Misses      10927    10973      +46     
- Partials      735      738       +3     
Impacted Files Coverage Δ
config/flags_payments.go 0.00% <0.00%> (ø)
config/flags_transactor.go 0.00% <0.00%> (ø)
core/node/options.go 0.00% <0.00%> (ø)
identity/registry/transactor.go 0.00% <0.00%> (ø)
mobile/mysterium/entrypoint.go 0.00% <0.00%> (ø)
session/pingpong/hermes_promise_settler.go 34.60% <76.66%> (+1.17%) ⬆️
requests/dialer_swarm.go 73.68% <0.00%> (-2.26%) ⬇️
tequilapi/endpoints/sse_handler.go 70.00% <0.00%> (-2.15%) ⬇️
nat/traversal/pinger.go 74.80% <0.00%> (-1.94%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cdaa020...2df76f7. Read the comment docs.

@tomasmik tomasmik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably just remove the cache, but it's fine either way.

@Guillembonet
Guillembonet requested a review from tomasmik May 16, 2022 09:35
Comment thread config/flags_payments.go Outdated
Comment thread session/pingpong/hermes_promise_settler.go Outdated
Comment thread session/pingpong/hermes_promise_settler.go Outdated
Signed-off-by: Guillem Bonet <guillem@mysterium.network>
@Guillembonet
Guillembonet merged commit 668c520 into master May 16, 2022
@Guillembonet
Guillembonet deleted the smart-auto-settle branch May 16, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow to configure fee options for auto settlements

4 participants