-
selsta
Is there a specified monero uri scheme?
-
selsta
-
selsta
because the one on wiki seems wtf, amount as float and the description claims atomic units
-
selsta
jberman[m]: do you want to squash your PRs?
-
rbrunner
I try to compile UkoeHB's multisig improvements:
monero-project/monero #7877
-
rbrunner
I run into a boost error quite early on:
paste.centos.org/view/116f1506
-
rbrunner
Something with boost::system::system_category() undefined, as far as I can see
-
rbrunner
Ups, have the same problem with master. What did change? Do I need to update boost? If yes, how?
-
UkoeHB
submodule thing?
-
UkoeHB
I have these written down:
-
UkoeHB
git submodule sync
-
UkoeHB
git submodule update --init --recursive
-
UkoeHB
git submodule update --init --force
-
UkoeHB
cmake -S . -B build -DMANUAL_SUBMODULES=1
-
vtnerd
its coming from epee, so probably not it
-
selsta
rbrunner: does it only happen with debug build?
-
rbrunner
I tried that first, yes. Release not yet tried.
-
vtnerd
looks like `${Boost_SYSTEM_LIBRARY} needs to be added to the dependency list in epee
-
selsta
-
rbrunner
thanks UkoeHB, selsta, vtnerd for all the hints. Compiling release right now, just to check it works. Will try that dependency added after that.
-
wfaressuissia
"I run into a boost error quite early on ..." it's reproducible only with boost <=1.65 and `-DBUILD_SHARED_LIBS=1` (enabled by default for debug target)
-
wfaressuissia
The above patch fixes it.
-
selsta
will PR it
-
selsta
Remaining PRs that need review: 7805, 7825, 7873/7874, 7881/7882
-
rbrunner
Hmmm, with that dependency fix I get basically the same error, but for another lib: Linking CXX shared library libepee_readline.so
-
rbrunner
-
rbrunner
That's now for UkoeHB's branch, not master
-
rbrunner
Unmodified release master compiles ok without that patch
-
selsta
-
wfaressuissia
also reproducible, the same `${..}` fixes it for `epee_readline`
-
rbrunner
Ah, yes, only a few lines below the other one, should have seen that myself. Anyway, noob here, stumbling through C++ stuff :)
-
rbrunner
Thanks
-
rbrunner
Maybe somewhere in the vast open-source software universe there is a tool that could find and fix all such errors once and for all automatically and systematically ...
-
rbrunner
Now it compiles successfully.
-
rbrunner
Hey, that's nice additional info, was that you, UkoeHB? :) Multisig info imported. *Number of outputs updated: 1*
-
UkoeHB
I don't think so
-
UkoeHB
I didn't make any behavioral changes to tx building code.
-
rbrunner
That was a praise, by the way, should there be any doubt. That's useful info, I am just a little surprised somebody took it on to improve it.
-
rbrunner
Started to test the MMS on your branch.
-
UkoeHB
-
UkoeHB
Cool looking forward to your results :)
-
selsta
UkoeHB: does this fix the security issue with multisig? or does that require more work?
-
UkoeHB
It fixes the security issue with making multisig wallets, but does not address the tx building issue (which is much harder to pull off).
-
UkoeHB
(and much harder to fix)
-
rbrunner
Might not even be worth it, with new code (hopefully) ahead within a reasonable timeframe anyway?
-
UkoeHB
actually maybe not harder to fix... probably similar or even fewer diffs
-
UkoeHB
rbrunner: no one has the answer to that yet. A) idk if anyone is enthusiastic about implementing multisig that supports Triptych, B) alternatives to Triptych are being researched...
-
rbrunner
Yeah, I am aware
-
rbrunner
I tested 2/2, and almost everything worked. At the end, after sending funds out of the multisig wallet again, the situation is strange:
-
rbrunner
The wallet that submitted has the correct balance, but does not tell about missing key images, and also does not report the mining of the tx
-
rbrunner
The other wallet is oblivious to everything, did not see anything happening, still has the old balance, even after refresh
-
rbrunner
Ups, I take all back, it just took an eternity for the wallet to notice the transaction.
-
rbrunner
So I can report that a full roundtrip worked for 2/2: Wallet building, funding, syncing, transfer out part of the funds from the multisig wallets, and finally syncing again
-
UkoeHB
Awesome! Thanks :)
-
UkoeHB
I wonder if there will be problems with M < N, since I killed `finalize_multisig()`
-
rbrunner
I will probably know in a few minutes, right now trying 2/3. If anything the MMS is at least good for testing multisig code
-
rbrunner
2/3 still works as well. Nice.
-
UkoeHB
yay! What about 2-of-4 (or 1-of-3, which I enabled in the new version)?
-
rbrunner
Wallet construction for 3/5 worked. (Don't try that at home without the MMS :)
-
sgp_[m]
Matrix room description updated
-
rbrunner
1/3 trivially fails because of an error check in the MMS code in simplewallet.cpp, 1 as number of required signers gets rejected there
-
UkoeHB
What's the error msg? Trying to find it
-
rbrunner
Error in the number of required signers and/or authorized signers
-
moneromooo
1 does seem a bit short of multi.
-
rbrunner
Well, not 100% sure anymore where the check is. Maybe also somewhere in wallet2.cpp
-
rbrunner
Well, yes, until now. It should be supported now, after the most recent changes by UkoeHB, in their PR
-
UkoeHB
rbrunner: in `mms_init()`, is `num_authorized_signers` the value 'N' and `num_required_signers` the value 'M', in M-of-N?
-
rbrunner
Er ... it's required/authorized, e.g. 2/3.
-
UkoeHB
Ok try latest commit
-
rbrunner
So ... yes?
-
rbrunner
Ok. That will have to wait for tomorrow however, night arrived here.
-
UkoeHB
Np :)
-
rbrunner
Alright, 3/5 worked up to sending funds out again. Perfect so far. Solid!
-
jberman[m]
<selsta> "jberman: do you want to squash..." <- all squashed :)
-
selsta
thanks
-
selsta
.merges
-
xmr-pr
7780 7805
-
selsta
.merge+ 7882 7881 7879 7878 7849 7848 7846 7845 7838 7822 7821 7832
-
xmr-pr
Added
-
selsta
-
selsta
If yes, please change at tx_amount "Float" -> "String" and "atomic currency units" -> "decimal units"
-
luigi1112
ok