-
Notifications
You must be signed in to change notification settings - Fork 179
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
Script: Add bond-calculator.py #1254
Conversation
30594c6
to
7a825d1
Compare
Huh. That's a pretty cool idea. Of course it's pretty hard to interpret the data anyway, but still. |
Sure, no rush with this.
Yep, that's an option too. I have no strong preference either way, no problem moving the PR if we prefer it.
|
Yeah fair points. There might be an argument to make that it could be folded in as a function of an existing script (that would make it even more accessible), though it doesn't fit entirely naturally into any one. |
It suddenly occurs to me as I'm looking at #1253 that |
It can work, and it would be cool to render this as tables/graphs. The downsides I can see (plus I don't like writing HTML, but that's just me):
|
I mean it would definitely be optional and on a separate page via hyperlink, just like sybil attack calcs today. But fair point that you can't run ob-watcher without being online, although you can run it without Bitcoin Core running. |
I was thinking about a startup option (e.g., |
7a825d1
to
41c47b5
Compare
I'm not sure that's even technically true; the orderbook lets you query for timed out counterparties, but the more important point is that several of the existing pages do fairly heavy computation/data load in the background, specifically the fidelity bond table and the FB sybil resistance calculations, then there's also charts and so on (though I never look at them because I always forget to install matplotlib). A simple calculator form to me is no big deal. But again, I really don't mind that much. |
You can absolutely be right, and I don't mind much either, the idea I think might be relevant is that this (and similar potentially expensive stuff) might not be desirable for people that just want to host a public orderbook for the orderbook, and not for the other fancier stuff. The "static" thing means that it is always the same calculation, it's not user defined (and if it's that heavy I think we could make the same argument for it. Also, this stuff would be in addition, so another argument could be not to add even more heavy computation). |
41c47b5
to
7121dce
Compare
So, to sum up the idea after some thinking, the underlying main goal is to give users as much information as possible before committing to a fidelity bond, and to give them a better understanding of what to expect from a
PulpCattel@c0f6401 adds a naive version of that using a quick and dirty Monte Carlo simulation (admittedly so I didn't have to work out a formula), it's just meant as a proof of concept. joinmarket-clientserver/jmclient/jmclient/support.py Lines 222 to 241 in 930c25f
These are a couple of examples from that branch (don't trust these numbers too much), it shows that creating a bond like the top fidelity bond in the orderbook, a gentleman with over 120btc locked for ~1 year, has (kinda unsurprisingly) a ~100% chances of being picked (thug life!) in a CoinJoin with 10 parties (i.e., the taker picks 10 makers) These calculations all use current defaults (i.e., 12.5%
|
Concept ACK. I like this being a script in main repo, not custom scripts, as is something useful for most people planning to run yield generator. |
Please consider making this a module in jmclient, with the script in |
help="For how many months to calculate the fidelity bond values, each month has its own stats (default 12)", | ||
default=12, | ||
) | ||
parser.add_option( |
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.
I'm guessing you already thought about this but we could do this without a file export ... but, it's another reason to fold it into ob-watcher which already does the quite obscure jobs needed to get the orderbook without a fully fledged JM bot ...
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.
Yeah, we discussed this a little earlier. The main problem I have with folding it into ob-watcher is requiring the ob-watcher machinery every time.
If we decide to go the way of a dedicated module, could we have both? That way this functionality would be everywhere, as dedicated script (using a .json
file), as ob-watcher extra page, and as RPC (I'm not sure here what the best solution would be to pass orderbook data).
Thank you, sounds good to me, I didn't think about the RPC (in retrospect seems obvious possible use case). |
7121dce
to
1637fbb
Compare
1637fbb adds a new helper module, is it more or less what you have in mind? (I'm not attached to it as is, please feel free to suggest reworks/refactors/what have you). Now that we have this module, if this is the direction we want to go, we could move the functions in |
jmclient/test/test_bond_calc.py
Outdated
interest = 0.015 | ||
exponent = 1.3 | ||
jm_single().config.set("POLICY", "interest_rate", str(interest)) | ||
jm_single().config.set("POLICY", "bond_value_exponent", str(exponent)) |
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.
Same problem as was mentioned in https://github.com/JoinMarket-Org/joinmarket-clientserver/pull/1254/files#r890587671
We address this same point in a bunch of other tests by a teardown/cleanup function, or you can do it directly in the test function etc.
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.
6f68498 removes these two config.set
, but it does introduce another one at the end (now that get_bond_values()
reset the exponent, we have to set it manually in the test to be able to compare the bond value result).
Added a reset at the end of the test.
Thanks, a thorough piece of work :)
Yeah it is. Also related, I think you already mentioned, but we should consider whether |
1637fbb
to
6f68498
Compare
Agreed. If nobody else does, I'll eventually open a follow-up PR to do that. |
I'm having error now:
|
My bad, unfortunately dateutil is third-party package, I missed it because it comes preinstalled in Debian (and Ubuntu too) with the python3-dateutil package. That's a bit sad because it was the cleanest solution by far, it can be replaced with something like: year = dt.year + dt.month // 12
month = dt.month % 12 + 1
return datetime(year, month, 1) The new version is clearly not a general one and would be fidelity bond specific, but I guess that should be okay. |
Add a new helper module to calculate fidelity bonds values and stats, and a script to use it. The goal is to give as much information as possible to the user before committing to a fidelity bond.
6f68498
to
72bf447
Compare
|
tACK. Apart from minor details, like updated docs, this looks good to me. |
OK, thanks. This seems complete now, merging. Hopefully we can extend this by putting it into the RPC. |
Add a new script that calculates and shows fidelity bond stats for many possible locktimes.
Depends slightly on #1253, but this works just the same without, those changes are here already to gather feedback.
I'm open to rework/rename any part of this, mostly I'm curious if you find it useful even in this basic form.
Further (potential) ideas, I'd prefer whenever possible to implement these in follow-ups, rather than here, to keep things simple:
bondless_allowance
), what are the chances of this bond given the orderbook?My math is spaghetti, please make sure it adds up.
Example