09:33:40 <m-relay> <t​heelevated:matrix.org> How to set up monero as a payment option on a ecom site? Any open source apps to do that? If possible can it also be multi currency for convenience sake
09:35:36 <m-relay> <e​ndor00:matrix.org> theelevated: the question is better suited for #monero-community-dev:monero.social
09:35:55 <m-relay> <e​ndor00:matrix.org> (This room is for developing Monero itself)
09:37:07 <m-relay> <t​heelevated:matrix.org> Sorry
16:34:32 <selsta> we will skip v0.18.3.0 and release v0.18.3.1 instead, if someone wants to contribute to reproducible builds please wait for the new tag. current plan is tomorrow if luigi is available.
16:35:46 <selsta> if someone has already built .0 please run it on your node and wallet, more testers are always good
17:57:16 <rbrunner> Uh, that max_blocks bug could be my bad ...
17:57:31 <selsta> rbrunner: not your bug I think
17:58:52 <sech1> Oh, it looks like Android gitian build fixed itself
18:00:10 <rbrunner> Hmm, could be that I had to merge in that #8355 improvement during the very long history of my "incremental" PR, and I might have missed that. Whatever :)
18:00:23 <rbrunner> Shows once again the value of good reviews, and good testing
18:00:35 <selsta> wait now i'm getting confused if my bug fix is even correct lol will double check
18:03:48 <rbrunner> Yeah, looks like the number of blocks is at the end? Confused myself now.
18:05:09 <selsta> will update it
18:05:48 <rbrunner> I think I remember now. *Two* parameters to that method were added, one boolean by me, and that block number by somebody else.
18:06:00 <rbrunner> Why would that need setting? It defaults to max, no?
18:06:18 <rbrunner> See the .h file
18:06:22 <selsta> the last false is currently setting it to 0, instead of max
18:06:25 <selsta> that causes issues during refresh
18:07:20 <rbrunner> That's the signature:
18:07:21 <rbrunner>    void refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blocks_fetched, bool& received_money, bool check_pool = true, bool try_incremental = true, uint64_t max_blocks = std::numeric_limits<uint64_t>::max());
18:07:21 <rbrunner>  
18:07:53 <rbrunner> And `try_incremental` has to go to false, as the comment explains
18:12:18 <rbrunner> Oh no, it looks like the signature of that `refresh` call is different now between master and release-0.18
18:12:34 <selsta> yes
18:14:26 <selsta> that also explains it why this bug wasn't triggered on master until now
18:14:57 <selsta> it only happens on release due to different signature
18:15:34 <rbrunner> Ufff
18:15:49 <selsta> my patch is correct for release branch, but it's more important to fix why they got swapped in the first place
18:16:36 <rbrunner> Within master alone, I don't think something got swapped. Like I said, 2 new parameters, and I think when merging I opted for the number of blocks at the end
18:17:07 <rbrunner> No idea yet how that got reversed on the release branch. Maybe me in a bad moment, maybe not ...
18:18:57 <rbrunner> I claim that on master we don't have a bug.
18:23:34 <selsta> updated https://github.com/monero-project/monero/pull/9011
18:23:42 <selsta> will double check if something else is also flipped
18:24:59 <rbrunner> Good idea. I am too tired right now to see fully through, but your new 9011 PR looks like the way to go for me.
18:25:32 <rbrunner> So this thing shot down a release? Crass.
18:25:47 <selsta> no, there's a second patch tomorrow
18:25:52 <rbrunner> Lol
18:25:53 <selsta> but this would have also been a bad bug
18:26:05 <selsta> breaking CLI refresh in some cases
18:26:21 <selsta> or all cases even
18:26:48 <rbrunner> Sounds strange. Will see tomorrow what you come up with :)
18:27:37 <rbrunner> I and j-berman tested a great deal on master with that stuff, being aware that it had the potential to break refresh which of course is very important
18:27:52 <rbrunner> Maybe release did not get the testing it deserved
18:28:58 <rbrunner> Ah, I think I misunderstand. The patch tomorrow is something else, not refresh *again*, right?
18:29:04 <selsta> yes
18:29:11 <selsta> unrelated to refresh
18:29:24 <rbrunner> Ok. But yeah, this is bad enough.
18:30:06 <rbrunner> And now we all blame C++ with its lax type rules.
18:31:08 <rbrunner> I mean, come on, a boolean gets accepted as uint64_t just like that, do I see that right?
18:34:58 <rbrunner> My "incremental" PR to the release branch doesn't have the number of blocks yet
18:34:59 <rbrunner> https://github.com/monero-project/monero/pull/8800/files#diff-19a145d91008c477e5d0b7890bb46ca0cb61fcd9917eb3910f6ed347c6ca6ab5
18:36:22 <rbrunner> Which I find a bit confusing, thinking about it ...
18:37:07 <rbrunner> Ah bah, I have to put that away now.
18:38:12 <selsta> https://github.com/monero-project/monero/pull/8941
18:38:42 <selsta> I didn't see that they got flipped and approved it
18:40:06 <rbrunner> So it might have been moneromooo who opted for the other way round as order for the two parameters, and did not adjust the simplewallet line?
18:40:42 <rbrunner> Which, because of the C++ peculiarities, did not result in a compile error?
18:41:29 <selsta> I just hope mooo didn't flip it because he something else flipped
18:41:52 <rbrunner> Don't understand
18:42:30 <rbrunner> Sarcasm? /s
18:43:48 <rbrunner> Looking at this wonderful confusion, I wonder what kind of bugs we will have in the final stretch of testing the Seraphis and Jamtis release
18:44:04 <rbrunner> If we ever get that far, that is :)
18:44:44 <selsta> everything should be good now with 9011 merged
18:44:55 <selsta> haven't been this confused in a long time lol
18:51:27 <rbrunner> Isn't this line in the RPC server still wrong?
18:51:27 <rbrunner> https://github.com/monero-project/monero/blob/f9b81a589e19f660ed803f505c8c2a1b309c3bed/src/wallet/wallet_rpc_server.cpp#L157
18:55:57 <selsta> yes
18:57:51 <selsta> will do a `grep refresh(` to make sure nothing else is left
18:58:20 <rbrunner> Spendid idea
18:58:32 <rbrunner> *Splendid
18:59:58 <rbrunner> I just hope nobody built something yet on top of this code which then would also have to flip
19:00:27 <rbrunner> That `refresh` method is public, after all
19:07:14 <selsta> we only merged it 2 weeks ago to release branch, doubt anyone already integrated it
19:10:08 <rbrunner> Alright
19:13:25 <selsta> .merge+ 9011
19:13:25 <xmr-pr> Added
19:24:56 <moneromoooo> I reckon it's just a bad merge, I might have mixed up one line. I don't recall flipping parameters on purpose for some particular reason.
20:31:32 <sech1> Is it the only place where refresh is called?
20:32:05 <sech1> I mean called with this number of parameters?
20:35:41 <selsta> once in simplewallet.cpp and once in wallet_rpc_server.cpp
20:35:53 <selsta> I check with grep both branches now and everything should be correct now
20:38:51 <sech1> I would temporarily rename that "refresh" to "refresh_blablabla" in all known places, and try to compile. Compilation will really show all places where it was used
20:42:17 <selsta> will try
21:10:27 <selsta> there are other places where the refresh function is called but they are setting default value
23:54:12 <luigi1111w> do we need to release again?