01:40:19 <nantuk[m]> Hi guys, as far as I know, xmr->btc atomic swaps cannot be implemented correctly until Monero adds transaction pre-signing (read this in a commit blog post). Does anyone know if this will be added in the next hard fork?
02:38:46 <nantuk[m]> This is the blog post: https://comit.network/blog/2021/07/02/transaction-presigning/
03:10:40 <UkoeHB> nantuk[m]: for transaction chaining (needed for xmr->X swaps) Monero would need to fork to something like seraphis (WIP next-gen tx protocol), which is at least 2 years away probably.
03:17:34 <nantuk[m]> <UkoeHB> "nantuk: for transaction chaining..." <- Oh all right, thanks
07:04:56 <kayabaNerve> get_fee_quantization_mask is static and will be until we have a major fee overhaul, so I can ignore it as an RPC value and instead write it as a constant myself... right?
07:06:32 <kayabaNerve> The only other consideration that may be relevant is if the unit fees UkoeHB proposed would be automatically applicable by wallets if they do operate with the RPC returned mask properly (setting the mask to 1 and the fee per kb to 1/2/... as well).
10:29:29 <r4v3r23[m]> jberman: thanks for taking on 7760 rewrite as node performance is greatly improved on it. will it make it into v0.18 or is there not enough time to get it in?
17:33:12 <jberman[m]> r4v3r23: Not necessarily working on a rewrite, just working through understanding it in full to review it. I think I'll be done in the next couple days but possibly not
17:45:28 <jberman[m]> <kayabaNerve> "get_fee_quantization_mask is..." <- It should be but note the upcoming fork modifies the fee algo, which you're going to want to make the RPC call to get_fee_estimate for, which returns the quantization mask anyway
18:14:03 <sech1> hyc I think we have a problem with lmdb in the latest master
18:15:46 <sech1> I'm debugging broken solo mining in xmrig when ZMQ is enabled
18:15:54 <sech1> this is what I get on monerod side: https://paste.debian.net/hidden/d3297b7c/
18:16:33 <sech1> so the blockchain height changes to 2644329 at 18:11:56.104 because new block was added
18:16:47 <sech1> but then it starts changing between 28 and 29 erratically
18:17:24 <sech1> xmrig calls get_block_template RPC as soon as it gets new block ZMQ notification, and get_block_template returns 2644328 instead of 2644329
18:17:55 <sech1> I did a workaround in xmrig: https://github.com/xmrig/xmrig/pull/3067/commits/db9069897df3e9ac750d57d501771f6200f8ff41
18:18:08 <sech1> but the root cause of what's happening is beyond my understanding
18:19:27 <sech1> and this is 100% reproducible. Every single new block - the same behavior
18:19:38 <selsta> and it doesn't happen in release-v0.17 ?
18:19:45 <sech1> release-v0.17 works fine
18:20:01 <selsta> how long does it take to reproduce the issue?
18:20:07 <sech1> 100% reproducible
18:20:14 <selsta> bisect.. again :D
18:20:17 <sech1> I just start mining solo xmrig with ZMQ enabled
18:20:22 <sech1> and wait for next block
18:20:23 <sech1> and boom!
18:21:11 <sech1> I can bisect, but maybe hyc can recognize this behavior?
18:23:22 <selsta> there is a different issue on reorgs, maybe they are related
18:23:45 <moneromooo> It's not the same thread, so each thread will have a different view of hte db until the txn commit. Did you account for this ?
18:24:17 <moneromooo> "release-v0.17 works fine" suggests yes, but... just making sure :)
18:25:30 <sech1> I didn't account for this
18:25:46 <sech1> so calling get_block_template immediately after is not safe?
18:25:59 <moneromooo> It depends after what.
18:26:15 <sech1> after handle_block_to_main_chain
18:26:27 <moneromooo> If calling on hte same thread, it'll be fine.
18:26:54 <sech1> so handle_block_to_main_chain sends miner notification to xmrig, xmrig calls get_block_template RPC immediately
18:27:03 <sech1> and this RPC accesses db on another thread, right?
18:27:07 <sech1> so it's not safe?
18:27:16 <moneromooo> If handle_block_to_main_chain hasn't commited the db txn yet, it's not safe.
18:27:39 <moneromooo> But a process creation etc is slow, so...
18:28:25 <moneromooo> If the miner notification is synchronous, it's definitely not safe. If async, it's not safe, but probably ok in practice.
18:28:37 <moneromooo> So not OK :D
18:29:02 <sech1> what process creation? It's ZMQ pub send
18:29:05 <moneromooo> This is probably an existing monero bug I think...
18:29:09 <sech1> so a few bytes over TCP connection
18:29:22 <moneromooo> Oh. I'm not familiar with those. I'm familiar with the execve one.
18:29:47 <sech1> it's sent in the end of handle_block_to_main_chain, already after m_db->add_block() succeeded
18:29:50 <moneromooo> So I'm guessing zmq exacerbates then.
18:30:20 <moneromooo> And I'm guessing there's a dtor that commits at scope end ?
18:31:00 <moneromooo> Hmm. Seems unlikely, IIRC commits are explicits... Anyway, worth checking.
18:31:41 <sech1> I'm bisecting now (going one month back at a time, recompiling and testing)
18:39:28 <selsta> if you start with master~300 you will have to do 8 compiles
18:39:39 <selsta> did it yesterday to find the glibc commit
18:42:04 <moneromooo> 9 at most... Sorry for pedantic, but... :D
18:42:21 <moneromooo> I think. Maybe. Off by ones are sooooo programmerese...
18:44:10 <selsta> hmm yes 9, 8 if you start with master~255
18:49:31 <sech1> commit 0f7b20a1ce5510ea730ce3ec6454ef4822309a51 (end of March) works
18:49:34 <sech1> I'll keep looking
18:52:06 <ooo123ooo1234567> https://github.com/monero-project/monero/pull/7937 - this is the only one directly related to src/rpc or src/cryptonote_core
18:55:10 <selsta> sech1: works as in doesn't have the issue?
18:55:17 <sech1> yes
18:55:49 <selsta> sech1: not sure if you have ooo on ignore but it might be https://github.com/monero-project/monero/pull/7937
18:55:59 <selsta> it would fit date wise
18:57:55 <sech1> I'm already bisecting around the date it was merged
19:22:02 <ooo123ooo1234567> <r4v3r23[m]> "jberman: thanks for taking on 77..." <- there are examples when less isolated changes merged without proper review and the main benefit of 7759/7760 isn't node performance (I didn't even measure speed of anything) and it must be included even before v0.18 and v0.18 has bigger troubles except broken connection code, not sure how explicit request for reviews will speed up it;
19:27:42 <r4v3r23[m]> <ooo123ooo1234567> "there are examples when less..." <- all i know is that nodes running 7760 are much more stable
19:30:16 <ooo123ooo1234567> r4v3r23[m]: unfortunately this level of understanding doesn't reflect all benefits from that patch
19:31:01 <r4v3r23[m]> im just a user, i dont need to understand. ever since using selsta's node my connections have been rock solid
19:32:13 <ooo123ooo1234567> it's like you're approaching some fundamental improvements and someone judging it by cover (code style), it only emphasize level of understanding of those who are reviewing
19:42:12 <kayabanerve[m]> <jberman[m]> "It should be but note the..." <- Mind elaborating? I wasn't aware of this.
19:42:27 <kayabanerve[m]> My issue with get_fee_estimate is it's not reproducible.
19:42:57 <kayabanerve[m]> My issue with the quantization mask is no one else on the planet uses it so I either need to craft custom code handling OR isolate it in Monero, which I know I can do so long as it's static.
19:43:27 <kayabanerve[m]> So if I'm already writing my own fee algorithm implementation, and the mask is static... that's when the question is raised of why am I calling g_f_e
19:43:58 <jberman[m]> https://github.com/monero-project/monero/pull/7819
19:44:06 <kayabanerve[m]> kayabanerve[m]: And this is a non-starter which can't be worked around without patches to monerod :/
19:44:07 <kayabanerve[m]> jberman[m]: Thanks
19:47:45 <ooo123ooo1234567> <r4v3r23[m]> "im just a user, i dont need to..." <- it's supposed to be used on every node, every time you're falling back to centralized solutions it harms in long term
20:04:40 <sech1> selsta yes, https://github.com/monero-project/monero/commit/83bb02745aa17921ab008c3196d317b991e6d47a is where the behavior changed
20:04:54 <sech1> previous commit works, this one doesn't
20:05:35 <sech1> or I'd rather say both work, it's just xmrig that needs to change how it calls get_block_template
20:06:17 <ooo123ooo1234567> sech1: wow
20:08:48 <ooo123ooo1234567> sech1: indeed, it's a try to justify whatever is needed to be justified, the same as with unbound: previously ubuntu 16 is sad, after ubnuntu 16 is 1 year after eol
20:27:36 <selsta> sech1: should we undo 83bb027?
20:28:02 <sech1> of course not
20:29:29 <sech1> btw p2pool isn't affected by this because it uses miner notifications sent from the same thread
20:29:54 <selsta> i mean if it's buggy behavior on monerod then reverting is better than a workaround on xmrig. if it was just an implementation issue with xmrig then it's fine.
20:30:03 <sech1> I don't think it's buggy
20:30:19 <sech1> get_block_template maybe should be fixed to wait for db lock before proceeding
20:30:36 <sech1> I already have a workaround for xmrig
20:31:18 <sech1> jberman[m] should look into it, I'm not familiar with the db code
20:35:32 <sech1> it's not buggy per se - get_block_template returns old block template if called before db is ready with the new data. So it's correct from RPC point of view
20:36:01 <sech1> but adding a lock there would improve it
20:36:51 <moneromooo> Yes, best to set a flag instead of calling, and call in cleanup_add_blocks if the flag is set.
20:37:15 <moneromooo> There's already a similar flag, forgot what I added it for, but similar thing.
20:38:54 <ooo123ooo1234567> <sech1> "but the root cause of what's..." <- How would you describe the root cause ?
20:40:08 <sech1> cleanup_add_blocks? I can't find it
20:42:27 <ooo123ooo1234567> <sech1> "but adding a lock there would..." <- or not improve, what is the root cause ?
20:43:56 * moneromooo goes look
20:43:57 <sech1> ooo123ooo1234567 you're on my ignore list. I just checked libera logs and I see I saved a lot of time by not seeing your comments. 99% garbage as usual
20:44:25 <moneromooo> cleanup_handle_incoming_blocks
20:44:55 <moneromooo> I'm reasonably sure this gets called for locally found blocks. Double check though.
20:45:31 <ooo123ooo1234567> sech1: There is a fix for ubuntu 16 compatibility , and comment with link to patch related to db, all are garbage ?
20:46:26 <ooo123ooo1234567> you're just following crowd, it isn't about garbage or not garbage and don't want to admit it
20:46:28 <sech1> yes, it's for embedded miner
20:46:39 <sech1> core::handle_incoming_block doesn't call it
20:47:01 <moneromooo> Right. It's called after it.
20:47:13 <moneromooo> prepare/handle/cleanup.
20:47:55 <moneromooo> If succesful. cleanup isn't called on error.
20:54:43 <ooo123ooo1234567> 1st question is how blackbox patch with lmdb locks in high layer of rpc changed something for rpc calls - there is certainly some overlook, 2nd question - are there any benefits from lmdb locks in rpc, it's certainly not long term solution for race conditions
20:56:57 <ofrnxmr[m]> "1st question is how blackbox patch with lmdb locks in high layer of rpc changed something for rpc calls - there is certainly some overlook, 2nd question - are there any benefits from lmdb locks in rpc, it's certainly not long term solution for race conditions"
20:56:57 <ofrnxmr[m]> -ooo123
20:56:58 <ooo123ooo1234567> zmq notification within src/cryptonote_core/blockchain.cpp is done before commit of tx into db, it's possible to reorder it, but why rpc patch with lmdb locks added some inconsistency if it was supposed to improve something
20:57:46 <ofrnxmr[m]> "zmq notification within src/cryptonote_core/blockchain.cpp is done before commit of tx into db, it's possible to reorder it, but why rpc patch with lmdb locks added some inconsistency if it was supposed to improve something"
20:57:46 <ofrnxmr[m]> --ooo123
20:58:07 <sech1> ooo123 clearly didn't read the code
20:59:02 <ooo123ooo1234567> sech1: if you is going to read code before submitting the next PR then I would be very happy
20:59:13 <ooo123ooo1234567> s/is/are/
21:00:32 <ooo123ooo1234567> sech1: as soon as someone will submit patch it will be possible to test: correct patch can't be written without code reading with high probability
21:01:53 <sech1> get_block_template holds m_tx_pool lock, and this lock is released in cleanup_handle_incoming_blocks, so all should be good. Need to wait for jberman[m] to clarify what is happening there
21:02:19 <ooo123ooo1234567> <sech1> "ooo123ooo1234567 you're on my..." <- libera logs doesn't contain referenced msg in reply (it's visible only on matrix side in the worst case)
21:02:41 <sech1> *Blockchain::create_block_template holds m_tx_pool lock
21:11:14 <ooo123ooo1234567> <sech1> "get_block_template holds..." <- read code, it's the best clarification tool
21:17:00 <ooo123ooo1234567> <sech1> "get_block_template holds..." <- I've found the root cause
21:17:17 <ooo123ooo1234567> indeed, it's due to those low level lmdb locks added into high level rpc
21:23:10 <ooo123ooo1234567> * level lmdb "locks, * locks" added
21:25:10 <ooo123ooo1234567> <ooo123ooo1234567> "1st question is how blackbox..." <- 1st question answered, 2nd question - it's more harm than benefits since db should be fixed from bottom to top without adding intrusive db "locks" into high layer blindly
21:25:57 <ooo123ooo1234567> <ooo123ooo1234567> "zmq notification within src/..." <- reordering will not help, it's clear now why rpc patch mad it something worse
21:26:03 <ooo123ooo1234567> s/mad/made/
21:26:03 <ooo123ooo1234567> s/mad/made/, s/it//
21:26:25 <ooo123ooo1234567> how is it garbage if these question helped with understanding of the problem ?
21:34:29 <sech1> "db should be fixed from bottom to top" -> usual garbage comment from ooo123, this is why you're on ignore
21:34:41 <sech1> I've found the root cause too
21:34:56 <sech1> db_rtxn_guard is locked in before the new data is fully commited to the db
21:35:33 <sech1> no need to rewrite db, and the db guard is correct there
21:35:44 <sech1> before, it would be executing in parallel with db update
21:41:32 <ooo123ooo1234567> <sech1> ""db should be fixed from..." <- it's impossible to use low level locks in high layer with encapsulated objects, in the worst case it could be deadlock / race condition / consistency
21:42:31 <sech1> the sequence of events uncovered with printf-debugging: https://paste.debian.net/hidden/f3e2f195/
21:42:40 <ooo123ooo1234567> <sech1> "no need to rewrite db, and the..." <- it's possible that db guard will be there in final version, but only after all layers below are fixed, not before
21:43:00 <ooo123ooo1234567> sech1: I've just read code to understand it
21:43:23 <sech1> so it's correct that get_block_template returns old data and the db guard prevented returning possibly incosistent data
21:45:23 <ooo123ooo1234567> sech1: consistency is expected across all api from daemon, it isn't consistent to see different from zmq and rpc
21:46:27 <ooo123ooo1234567> events may be asynchronous (it's needed for speed), but they should be consistent in order to be reliable for external user
21:49:06 <sech1> this can be fixed by moving db_rtxn_guard after m_tx_pool.lock(); inside create_block_template
21:49:08 <sech1> trying it now
21:53:28 <hyc> it would certainly be bad practice to take a db lock long before it's needed
21:53:53 <ooo123ooo1234567> sech1: it means move db_rtxn_guard (added blindly in 7937) from core_rpc_server::on_getblocktemplate into Blockchain::create_block_template
21:54:15 <sech1> yes, it takes the db lock and then waits for m_tx_pool.lock() for a long time
21:54:43 <sech1> yeap, moving db_rtxn_guard to the right place works like a charm
21:54:46 <sech1> PR soon
21:55:41 <ooo123ooo1234567> sech1: wow, so once you've found the root cause it was possible to fix it; so unexpected
21:56:31 <ooo123ooo1234567> sech1: 7937 is still good ? any new arguments ?
21:57:53 <ooo123ooo1234567> hyc: it isn't about good/bad practice, it's about understanding what is going on; in the worst case bad/good practice may be not appropriate
22:01:32 <ooo123ooo1234567> <sech1> "so it's correct that get_block_t..." <- so correct that it was required to move "lock" from rpc into blockchain class
22:01:32 <sech1> regarding 7937, it needs to be checked for other places where lock duration can be reduced
22:02:09 <ooo123ooo1234567> https://github.com/monero-project/monero/pull/7936, it should be replaced with more consistent methods for core/blockchain classes since it deeply depends on their internals
22:02:27 <ooo123ooo1234567> and not with "locks" in rpc
22:03:08 <ooo123ooo1234567> sech1: no, reverted and then added separate methods for core/blockchain which will hold appropriate locks where it's needed without inconsistency
22:05:50 <ooo123ooo1234567> sech1: it isn't about performance optimization to reduce time for lock being held, but about consistency in api (it's much bigger problem in scope)
22:06:05 <ooo123ooo1234567> > <@sech1:libera.chat> regarding 7937, it needs to be checked for other places where lock duration can be reduced
22:06:05 <ooo123ooo1234567>  * it isn't about performance optimization to reduce time for lock being held, but about consistency in api (it's problem with much bigger scope)
22:08:28 <sech1> who said anything about performance optimizations? I meant lock scope, not duration. This issue was particularly because lock order was wrong. Every place where DB read lock is taken needs to be examined to check where the data is being updated and what locks are held in those places.
22:08:47 <ooo123ooo1234567> <sech1> "regarding 7937, it needs to be..." <- just think: core is an abstract lib with all hard calls that could be used by different apis (rpc / zmq / ...), these calls should be made consistent within core, not within rpc
22:09:33 <ooo123ooo1234567> * / ... / p2p), these
22:10:44 <sech1> https://github.com/monero-project/monero/pull/8383
22:10:55 <sech1> selsta moneromooo ^
22:12:54 <ooo123ooo1234567> sech1: 7937 added "locks" blindly, now you're placing one of them into right place and asking for review and ignoring bigger issue which is clear from problem investigation; facepalm
22:15:10 <ooo123ooo1234567> funny that 7937 started from https://github.com/monero-project/monero/pull/7936, which started from few irc comments; just facepalm
22:19:48 <sech1> There's no bigger issue. Added you on ignore again, you're unbearable. Look, we all know that you like facepalming and solving everything by rewriting from top to bottom. Just stop. While you're facepalming, I solved the issue at hand.
22:23:06 <ooo123ooo1234567> sech1: zmq is an api but called within core, rpc is an api but called outside of core; at the same time someone is adding db "lock" into an api (rpc) in order to have consistent view on core calls, but different api returns different view; it is inconsistent behaviour
22:26:17 <ooo123ooo1234567> <sech1> "There's no bigger issue. Added..." <- solved one small scope issue, but advocated for preserving of incorrect patch + advocated for not wise fixes in long term for similar issue + not admitted that 7937 is a wrong way; it's more harm than good
22:27:56 <ooo123ooo1234567> the problem is that it isn't just you, but many things at once, and when I'm talking about one of them it leads to pointless ping-pong where no one is responsible for anything
22:29:07 <ooo123ooo1234567> sech1, even if you're acting in good faith it allows to hide some problems, it's bad in long term
22:31:26 <ooo123ooo1234567> <sech1> "There's no bigger issue. Added..." <- btw, "it's from bottom to top"; hahahahaha
22:32:08 <ooo123ooo1234567> <sech1> "There's no bigger issue. Added..." <- and it isn't applicable for everything, in that particular it's possible to do small careful changes before full rewrite, but not blind changes in high layer without understand low layer
22:59:25 <MeowingCat> what is wrong with wallet2???
22:59:50 <ooo123ooo1234567> s/"//, s/"/, nobody is doing rewrites from top to bottom/
22:59:51 <MeowingCat> Windows Defender saw that as a virus
23:00:14 <MeowingCat> idk why now
23:00:27 <MeowingCat> but it was not recognizing wallet2 as a virus
23:00:30 <MeowingCat> but it did
23:00:58 <ooo123ooo1234567> it looks like reproducible problem, try to bisect
23:02:12 <MeowingCat> maybe about runtime security but it is interesting because
23:02:13 <MeowingCat> it was same
23:02:27 <ooo123ooo1234567> <sech1> "There's no bigger issue. Added..." <- btw, +1 for finished bisection, it was useful work anyway
23:02:28 <MeowingCat> my code was same to yesterday
23:02:42 <MeowingCat> and this morning Windows Defender decided it is a virus
23:03:03 <MeowingCat> i rebuild it after that
23:03:06 <MeowingCat> it was working
23:03:15 <MeowingCat> but now it is not getting blocks
23:03:23 <MeowingCat> Monero made me really crazy
23:04:06 <UkoeHB> MeowingCat: can you try to put more of your thoughts in one comment? Instead of 100 separate sentence fragments. Also, filter out the ones we don't care about
23:04:53 <MeowingCat> you can try not to be rude instead
23:06:23 <ooo123ooo1234567> MeowingCat: that's just personal spam protection before adding someone into personal ignore list, not global rule
23:10:20 <ofrnxmr[m]> Still. Put your thoughts together before hitting the enter button.
23:10:20 <ofrnxmr[m]> Tip: use periods [ . ] MeowingCat: