<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

<e​ndor00:matrix.org> theelevated: the question is better suited for #monero-community-dev:monero.social

<e​ndor00:matrix.org> (This room is for developing Monero itself)

<t​heelevated:matrix.org> Sorry

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.

if someone has already built .0 please run it on your node and wallet, more testers are always good

Uh, that max_blocks bug could be my bad ...

rbrunner: not your bug I think

Oh, it looks like Android gitian build fixed itself

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 :)

Shows once again the value of good reviews, and good testing

wait now i'm getting confused if my bug fix is even correct lol will double check

Yeah, looks like the number of blocks is at the end? Confused myself now.

will update it

I think I remember now. *Two* parameters to that method were added, one boolean by me, and that block number by somebody else.

Why would that need setting? It defaults to max, no?

See the .h file

the last false is currently setting it to 0, instead of max

that causes issues during refresh

That's the signature:

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());

 

And `try_incremental` has to go to false, as the comment explains

Oh no, it looks like the signature of that `refresh` call is different now between master and release-0.18

yes

that also explains it why this bug wasn't triggered on master until now

it only happens on release due to different signature

Ufff

my patch is correct for release branch, but it's more important to fix why they got swapped in the first place

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

No idea yet how that got reversed on the release branch. Maybe me in a bad moment, maybe not ...

I claim that on master we don't have a bug.

updated monero-project/monero #9011

will double check if something else is also flipped

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.

So this thing shot down a release? Crass.

no, there's a second patch tomorrow

Lol

but this would have also been a bad bug

breaking CLI refresh in some cases

or all cases even

Sounds strange. Will see tomorrow what you come up with :)

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

Maybe release did not get the testing it deserved

Ah, I think I misunderstand. The patch tomorrow is something else, not refresh *again*, right?

yes

unrelated to refresh

Ok. But yeah, this is bad enough.

And now we all blame C++ with its lax type rules.

I mean, come on, a boolean gets accepted as uint64_t just like that, do I see that right?

My "incremental" PR to the release branch doesn't have the number of blocks yet

Which I find a bit confusing, thinking about it ...

Ah bah, I have to put that away now.

I didn't see that they got flipped and approved it

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?

Which, because of the C++ peculiarities, did not result in a compile error?

I just hope mooo didn't flip it because he something else flipped

Don't understand

Sarcasm? /s

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

If we ever get that far, that is :)

everything should be good now with 9011 merged

haven't been this confused in a long time lol

Isn't this line in the RPC server still wrong?

yes

will do a `grep refresh(` to make sure nothing else is left

Spendid idea

*Splendid

I just hope nobody built something yet on top of this code which then would also have to flip

That `refresh` method is public, after all

we only merged it 2 weeks ago to release branch, doubt anyone already integrated it

Alright

.merge+ 9011

Added

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.

Is it the only place where refresh is called?

I mean called with this number of parameters?

once in simplewallet.cpp and once in wallet_rpc_server.cpp

I check with grep both branches now and everything should be correct now

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

will try

there are other places where the refresh function is called but they are setting default value

do we need to release again?