The source is building for the first time now and I'm sure I'll need to watch some educational videos for a while before I know what yall are talking about most of the time.

Well that built faster than most other projects I've worked on, very nice.

im implementing Base58 of Monero

i understand padding to 11 characters

what is happening to last 8 bytes???

@sech1 Do you know if the leak fixed in tevador/RandomX #246 was actually happening in monero applications ?

jeffro256[m] it's happening every time RandomX can't allocate memory using huge pages (which is all machines that didn't set up huge pages). But in case of Monero, it's a one time leak of the small size.

why are Monero integers big endian???

Varint? They're not big endian, they're varint

and they're kind of little endian

if you found big endian somewhere in Monero's codebase, it's most likely network byte order (=big endian)

C#'s BigInteger is working with little endian on x86

when i parse private key from hex

I don't see how it's connected to your previous question

and derive Monero keys

they are being wrong if i don't reverse the private key

These are not numbers, they're ed25519 group elements

oh damn group elements

of course the encoding could've been little endian, but the library Monero uses has big endian encoding for them

ge_p3_tobytes()?

private keys are not group elements

they are little endian integers

how are they not group elements if they're used directly in all the Monero's cryptography?

they are members of the group of integers 0-l

but def not ed25519 group elements

hi

sech1 public keys are group elements because they're points (multiplied against the generator). Private keys are just the big random number that gets multiplied against the generator

So you're right it wouldn't be encoded like a normal integer, but I don't think they're group elements

But anyways, nice find on RandomX, cheers !

Can anyone point me to a code implementation of the compressed point decoding process from Zero to Monero section 2.4.2? I've written a version of it whose output always matches another decoding function, but I tweaked it in some weird ways and I'd like to check my work. Anything in python or c/c++ would be especially helpful.

I think you want ge_frombytes_vartime

github.com/FiloSottile/edwards25519…80f98a0424921c66e85d/scalar.go#L101 is what I use in my go implementation, if I understand you correctly

jeffro256[m]: it is encoded like a normal integer, just backwards

Bytewise

wernervasquez[m]: he’s looking for the opposite direction (decoding)

Thanks all, ge_frombytes_vartime looks like what I'm after. I'll poke around crypto-ops a bit and try to get familiar with things.

UkoeHB spackle_xmr : I linked to the wrong thing entirely. Here is what I use for encode and decode of points github.com/FiloSottile/edwards25519…0424921c66e85d/edwards25519.go#L123

> <@wernervasquez:matrix.org> UkoeHB spackle_xmr : I linked to the wrong thing entirely. Here is what I use for encode and decode of points github.com/FiloSottile/edwards25519…0424921c66e85d/edwards25519.go#L123

>

> github.com/FiloSottile/edwards25519…0424921c66e85d/edwards25519.go#L145

Thanks, I'll give that a look as well.

Hah, just saw that the code reference is marked in the page margin of ZtM. I'll remember to keep my eyes open in the future.

<sech1> "#7760 needs serious and thorough..." <- Did you try to run tests ?

<sech1> "see my comments there" <- "monero-project/monero #7760#discussion_r88455935", subjective comment about naming

"monero-project/monero #7760#discussion_r884563318", this is structure for internal use, it isn't class; subjective comment about naming

"monero-project/monero #7760#discussion_r884563950" subjective comment about naming

"monero-project/monero #7760#discussion_r884568328" comment about performance drop due to location of static objects within functions; is it measurable drop ? old code doing the same, so it's at least not worse than before

"monero-project/monero #7760#discussion_r884573083" comment about unimportant performance drop, old code is doing the same

"monero-project/monero #7760#discussion_r884583691" did you try to read code ? it's thread safe

"monero-project/monero #7760#discussion_r884585709" subjective comment about not vs !

"monero-project/monero #7760#discussion_r884586499" subjective comment about not/and vs !/&&

"monero-project/monero #7760#discussion_r884588779" did you try to read code ? it's thread safe

"monero-project/monero #7760#discussion_r884590240" subjective comment about mixture of !/&& and not/and

"monero-project/monero #7760#discussion_r884590901" did you try to read code ? it's thread safe

prove that it's only ever called from one thread at a time

"monero-project/monero #7760#discussion_r884595212" in the worst case that std::random_device will fail to call rdseed/rdrand and will throw c++ exception, it's unimportant note about performance; old code is doing the same, any questions about this stupid addition to reviewers of this PR monero-project/monero #5893

Okay, I see you don't want to cooperate

"monero-project/monero #7760#discussion_r885001542" all questions to reviewers of this monero-project/monero #5893

you missed this std::random_device issue even though you were rewriting the whole file

so much for the "perfect" daemon

I will not approve this PR until you fix it

sech1: Did you reply to all github comments or not yet ?

sry, Did I reply to all github comments or not yet ?

don't reply, fix the code

sech1: confirm that I've replied to all comments and we can discuss deeper all issues, starting from the most important to less important

I don't have time right now

This random engine change was introduced as a security fix so there should be a separate discussion about removing it again.

sech1: yes, it's; every place you mentioned is called only from entry point with acquired state.lock

I didn't do the full review yet. I literally spent less than an hour to just get familiar with the code

sech1: I want, but you're not right in everything; I didn't look into github yesterday, only opened it now

so no, I didn't "try to read the code"

sech1: vtnerd did the same and failed in all cases

sech1: Do you want me to fix code style changes before you will look deeper or what ?

if you call your PR "perfect" and at the same time leave unoptimal code that I found, you're lying

fix it or I won't approve it

I haven't started the full proper review yet

because it will take many hours

sech1: Call at least 1 place where that code is worse than old one ?

Some places were not touched, since they are not important

but all important problems are solved there

sech1: It's easy to fix all code style changes, but will you look deeper or not ?

github diff shows that everything was touched

I said I don't have time right now

I'll start this week

<sech1> "because this is unacceptable" <- It's unacceptable for reviewer to write shallow subjective comments about code style, without even reading code, running tests

Instead of starting personal attacks, fix the code

Code style is important

I don't want to see the zoo in the code

sech1: any link to code style that you respect ?

I don't want to change code style at preference of every reviewer

Change to what is similar to the existing Monero code

Not what I respect or prefer!

especially incompetent that only write comments about code style

As I said, if you're too stubborn to cooperate, I will not aprove it

sech1: Can you paste link to reference code style ?

So you're calling me incompetent now?

google.github.io/styleguide/cppguide.html, this ok ?

Then who missed this std::random_device glaring issue?

sech1: No, that's about previous reviews

Which I found after quick look

not from you

sech1: I didn't miss, it. I even wrapped that random_device into separate random_delay function in order to remove it completely

"monero-project/monero #8365" this PR is placebo, not needed

sech1: That's the problem: every reviewer pick something not important, but don't read code

Entry point is tests, there are at least 3 deep problems

for the 100th time: I didn't start the review yet!

I just looked at the code to see if it's worth to spend more time. Found one serious issue and two performance issue which you dismissed, yet called the code "perfect"

<sech1> "because this is unacceptable" <- you've written conclusion without reading code, how does it work ?

is it acceptable ?

I read the code of random_delay function

and called it unacceptable

sech1: "one serious issue" which one ?

random_delay

you saw it and didn't fix it

while rewriting the whole code

so fix it

or we don't have anything to talk about

I actually stopped looking at the code when I found it

sech1: I've leaved it as is, it's not worse than in old code; And this code fixes real correctness issues, unlike unimportant performance issues, you're talking here

because I'm not sure in overall quality of your PR now

sech1: too early then

sech1: I'll change random_delay and performance issues, without code style changes, ok?

is it enough ?

I'm sure you will spend much more time on code reading, than this shallow comments

enough for me to pull the code, compile it and run tests. And then run it under debugger to see how it works live

That's why it isn't important to do any polishing

polishing is important

or the next guy who looks at it will be confused

and code style is important too

have you ever worked in a big team?

sech1: That PR was written almost 1 year ago and nobody reviewed it, And now when jeffro256 added placebo PRs which introduced merge conflicts, someone is reviewing it

I'm reviewing it voluntarily

ooo123ooo1234567: we're not talking about anything subjective. Have you ever learnt how to code and how to contribute to a project?

And I can just walk away and let it rot if you keep begin so stubborn

*being

sech1: link to code style so that it isn't preference of every reviewer

I can link you to my comment abount naming

which is not my preference, rather what is used in other parts of Monero code

sech1: That's also a problem, there are many paid incompetent developers, but it's you who is reviewing it

who ever writes && => and, || => or and then mix these two styles?!?! unbelievable. Sech1 has already suggested you how to fix your code.

sech1: Monero code is written differently in different places, there is no code style; this code style is ok for you google.github.io/styleguide/cppguide.html ?

So you're adding one more place where it's written in N+1-th code style. Awesome (not)

find the most common code style that's already being used in most important files, and use it!

there's no step-by-step guide I can link you to

sech1: I'm about non subjective doc rather than step-by-step guide

ooo123ooo1234567: I'm trying to figure out your patch. Is it all by yourself?

OCREAT: yes

Google C++ Style Guide is not ok

doesn't match what Monero code uses

ooo123ooo1234567: okay, because it does not seem so. You can't even figure out what a writing style is, pretty useless to argument against Monero code style.

<sech1> "And I can just walk away and let..." <- complaining about code style without running tests, is it being stubborn or not ?

sech1: I suggest documenting the monero code style

OCREAT: scammer, is it you ?

There is no single monero code style. Please try to follow the style of whatever you're modifying. For new code, try something similar/sensible.

pretty boring argument, c'mon you can do better ooo123ooo, but it's not the right place to talk over

Siren[m]: It's useless goal, it's better to pick new one and follow it during all new PRs

no you don't

use existing code style

mooo gave a good suggestion

ooo123ooo1234567: If you're confused about it, perhaps there's use for it

I hope we can all strive to improve on the existing code style(s)/non-style...

according to github.com/monero-project/monero/blob/master/docs/CONTRIBUTING.md there should be a documented code style, but not sure if this was overlooked

Siren[m]: Find at least 1 existing code style which is defined in all cases, link

Well, that non-style has served pretty well for many years already

Siren[m]: You're arguing about the most shallow / unimportant thing, instead of reading code; facepalm

What we discuss here right now is definitely not "unimportant" IMO

And not facepalm-worthy

he said he hasn't started reviewing it yet. would you rather hear the first impressions now or in a week?

I'm not looking forward to petty formatting bullshit. As long as people are sensible and fairly consistent, it should be enough. Strict submission to rigid rules is in noone's interest except those who get off on punctilliousness.

ooo123ooo1234567: You sound like you don't care about your code quality at all and you're trying to get your code merged just so it is merged

Siren[m]: ding ding we have a winner

Winner in what regard?

Accuracy award?

Siren[m]: wondering if he's included any 0day on their code

:)

Siren[m]: The hardest part is write correct code and check it's correctness. Code style is easy to change, can be done even before merging.

its*

c'mon, you don't even speak english, whoever is this?

Siren[m]: I think ooo is willing to change the code style but doesnt want to waste time of style if there are other issues

It looks like I'm talking with linguists rather than programmers here

OCREAT: linguist ?

Seems like style adds a lot of delay (eg the serialization PR)

I'm intentionally write code in that style in to emphasize how incompetent people

vtnerd failed in all cases

It LoOkS lIkE i'm TaLkInG wItH lInGuIsTs RaThEr ThAn PrOgRaMmErS hErE

I can cope with different code style, for now, but you need to fix other issues (random_delay and host_count functions)

But I can change code style easily, others can change code style many times, but still write incorrect code

you clearly didn't work in a team

OCREAT: Can you stop?? stay on topic

correct code can be correct all the way you want, but if no one understands it or can read it, it's unmaintainable

and it will be scrapped

OCREAT: mj-xmr, use your real account

ooo123ooo1234567: I'm not about the style alone (although imo not adopting a style standard makes the code harder to read), reading this whole convo gives me that idea

sech1: I'm sure it isn't much different from your p2pool code

not much

> <@ooo123ooo1234567[m]:libera.chat> > <@OCREAT:libera.chat> It LoOkS lIkE i'm TaLkInG wItH lInGuIsTs RaThEr ThAn PrOgRaMmErS hErE

>

> mj-xmr, use your real account

Hello. Thanks for dragging me to another beef.

But I'm already fed up with your shit.

but renaming std::string to string_t, why?

just for the sake of renaming?

like no other programmer knows what is std::string?

That's a hell of an idea, yeah

Please don't ping me ooo123ooo1234567[m]

I don't talk with idiots/

mj-xmr[m]: you can /ignore people

Oh how many times I already did this...

This parasite goes around every blocks and bans

He makes this whole space a dirty toilet so full of him.

sech1: I'm sure that anyone who spent time on verification of that code (not as linguist) wouldn't even complain about code style. It was easier to write with renames, just it.

mj-xmr[m]: pls just block and move on, we gotta stay at least kind of on topic

sech1: No, I don't care about code style. It's easy. Give me guide, i'll change whole code according to it.

UkoeHB: Does it mean he won't ping me again? No.

But here we go.

sech1: In current environment I would say nobody understands even code written with some code style, check any code from vtnerd.

I can't give you the guide. Use common sense if you have it. mooo told you already "Please try to follow the style of whatever you're modifying."

monero-project/monero #7999/files#diff-af8e6ea1b236e77edb1a1498dc9a47769a1bec777ce99b98b7ad98d11e4c4c5bR72 oh yeah, wtf is this?

ooo123ooo1234567: Traslation: no problem sech, I will.

<sech1> "I can cope with different code..." <- ok, will do these changes + merge conflicts

> <@ofrnxmr[m]:libera.chat> > <@ooo123ooo1234567:matrix.org> No, I don't care about code style. It's easy. Give me guide, i'll change whole code according to it.

>

> Traslation: no problem sech, I will.

This is another sock puppet, just to let you know.

also w is.

perfect-daemon looks like "bad-coded daemon" :/ \: lol

but I think you figured it out by now.

> <@mj-xmr[m]:libera.chat> > <@ofrnxmr[m]:libera.chat> > <@ooo123ooo1234567:matrix.org> No, I don't care about code style. It's easy. Give me guide, i'll change whole code according to it.... (full message at libera.ems.host/_matrix/media/r0/do…14041d9be400b981e58bd99da3bb403a2fa)

I meant `w`

OCREAT: Oh yes.

And very unprofessionally.

not only "bad"

"unprofessional daemon"

I'm happy you said it, not me :)

there was on topic patch discussion and people derailed it from the side.. nice

Anyway, sorry for breaking your fun. Bye

big shoutout to whoever will handle this situation

Don't ping me with bullshit.

how about everyone who isn't involved in reviewing this patch keeps out of this discussion?

OCREAT: sech1: will and I will help.

OCREAT: Seems handled to me.

selsta: okay, sorry, we can move on

selsta: I'm actually involved.

> <@mj-xmr[m]:libera.chat> > <@selsta:libera.chat> how about everyone who isn't involved in reviewing this patch keeps out of this discussion?

>

> I'm actually involved.

You can't even debug bash script; facepalm

And I can tell you that this PR doesn't meet:

- industry standards

- Monero standards

...

mj-xmr[m]: See here.

OK. Cya all.

<OCREAT> "Siren: wondering if he's..." <- That's task of reviewer to check.

> <@mj-xmr[m]:libera.chat> And I can tell you that this PR doesn't meet:

> - industry standards

> - Monero standards

facepalm

<Siren[m]> "I'm not about the style alone (..." <- incompetent people that don't write complex code are caring about code style; code can be complex even with adoption of any "code style"; it's about dependencies, not about names.

And number of dependencies can be reduced even without "code style" changes.

<OCREAT> "perfect-daemon looks like "bad-..." <- Call at least 1 problem related to correctness introduced by that patch.

Complex code needs caring about code style more than any other, but that's probably just me ...

this ^

What really matters for Monero is: 1) robustness of code 2) a minimum of readability 3) proof of working with unit tests

And naming of course, naming

"write your code like the next person maintaining it will be a psycho who knows where you live" (c)

FWIW, I like ooo123's use of tests to show the fix. That's nice.

that I agree with

<UkoeHB> "Siren: ding ding we have a..." <- facepalm

> <@mj-xmr[m]:libera.chat> > <@ofrnxmr[m]:libera.chat> > <@ooo123ooo1234567:matrix.org> No, I don't care about code style. It's easy. Give me guide, i'll change whole code according to it.... (full message at libera.ems.host/_matrix/media/r0/do…e7aabb7ffc4826c047f0381a76db0662265)

moneromooo: They can be reused by a more professional developer. That's even nicer.

> <@w[m]:libera.chat> > <@mj-xmr[m]:libera.chat> > <@ofrnxmr[m]:libera.chat> > <@ooo123ooo1234567:matrix.org> No, I don't care about code style. It's easy. Give me guide, i'll change whole code according to it.... (full message at libera.ems.host/_matrix/media/r0/do…e31c74e1e579e16cac7a26dcfbf2dba44df)

I secured the unit tests on my local hard drive, in case imperfect-demon wants to threaten you with removal of them.

mj-xmr[m]: facepalm

<rbrunner> "And naming of course, naming" <- Yes, and LoLcAt CaSe.

Not sure what you, but anyway, I meant that seriously

*you mean

I thought you were joking.

Sounded like it.

<Siren[m]> "I'm not about the style alone (..." <- Why are you supporting that scammer ?

Obviously variables like `a` or `b` would not cut it.

So my take on this is, that a professional programmer, be it sech1 or somebody of comparable experience, takes the conclusions learned from that PR, and breaks them into smaller pieces. Only small PRs are able to be reviewed and merged for many reasons, that I shouldn't have to tell you. I wrote it all in sech1 PR.

Even if my funds are blocked, I will help in reviewing such managable PRs.

mj-xmr[m]: you don't know what you are talking about; facepalm

mj-xmr[m]: Aha, your funds are blocked; that's why you're talking so much

" Only small PRs are able to be reviewed and merged" Don't agree for once, and that would be a grave problem for our code base if really true

I've not read yet msgs from monero-community

It's not tolerable, that the original PR is left unmaintained for a year, exposing the vulnerabilities to bad actors for a year.

With this problem, was it real, we could flat-out forget completely about introducing Seraphis

rbrunner: That's how it works in practice. I've had many PRs open here and only small ones were even taken into account. Sorry, but that's how it is.

rbrunner: For Seraphis, I think that nobody will argue, that we have to make an exception.

mj-xmr[m]: incompetent developer write bullshit, which is merged into repo, but this scammer blaims correct unreviewed code in problems; facepalm

That happening to you escaped my attention. Somehow, at first impression, it sound improbable.

mj-xmr[m]: So instead of riding the dead horse, please focus on progress and PLEASE invest time into stable and professional people.

Otherwise you're left hanging when that unstable one has a bad mood.

And don't tell me it's "unrelated"

rbrunner: I actually **keeps happening** to me, not just happened. But no problem. There are more important things and I also have to adapt to reality, not the other way around.

Large PRs are possible if you have competent reviewers for this. Small PRs just increase the likelihood a PR will get reviewed properly.

Yes. But even competent reviewers will get tired and demotivated after some point. Especially after having to deal with sock puppets.

That's the point :)

And I'm not calling myself a competent reviewer.

Yes, that's why it's usually a good strategy to be somewhat careful if a good potential or actual reviewer comes around ...

I'm only trying to save the project.

We don't have them now. Except vtnerd and sech1 from time to time I think. But mostly we don't. Though I don't keep up with the PR list anymore so apologies if I'm missing someone ^_^

rbrunner: A reviewer is GOLD.

mj-xmr[m]: hahahahahaha; scammer is saving the project

moneromooo: everybody knows how long the PR list is.

mj-xmr[m]: the only thing that you cares about is your money, and nothing else.

s/cares/care/

It seems jberman could have a good career ahead of him, also as a reviewer.

Yes.

Ah yes, I remember good comments on my recent patches.

rbrunner: reddit.com/r/Monero/comments/umgew1…e=reddit&utm_medium=web2x&context=3

Here's an example where people ask me how many money they have to throw at me to fix Monero's issues, and instead I recommend them to fund the other devs, jberman[m] included. imperfect-demon excluded.

<moneromooo> "FWIW, I like ooo123's use of..." <- People learn only from challenges and these tests is good entry point for anyone to learn problem and check whether it was fixed. Any fiction literature in comments only help to simulate understanding via parroting of words from text in the worst case.

Now were you able to calculate perhaps how much time exactly did you spend cumulatively on trying to merge 7760, instead of trying something better, like I proposed in 8365?

<OCREAT> "github.com/monero-..." <- link to source with compile time strings for c++11 templates

You also can't tell me that the reason it can't be merged is that "we can't find reviewers". We have a few of them but they can't be demotivated with:

- too large PRs when they can be smaller (unlike Seraphis)

- stubborn contributors (if, as a contributor you don't need a reviewer, then it means you need to start your own project)

- sock puppets during every freaking meeting

The train won't drive if you don't build tracks first.

> <@mj-xmr[m]:libera.chat> You also can't tell me that the reason it can't be merged is that "we can't find reviewers". We have a few of them but they can't be demotivated with:... (full message at libera.ems.host/_matrix/media/r0/do…994d1e8a15dcc68a3d7462ec1add6131feb)

> <@w[m]:libera.chat> > <@mj-xmr[m]:libera.chat> You also can't tell me that the reason it can't be merged is that "we can't find reviewers". We have a few of them but they can't be demotivated with:... (full message at libera.ems.host/_matrix/media/r0/do…36d552d3d72f2512c31da9ff18f227a61c0)

What are your contributions?

> <@w:monero.social> mj thinks i am or that i know ooo

>

> lol. why so tin foiled

that man is either scammer or pathological lier or certainly has bad opsecs so that deanonimized himself

> <@mj-xmr[m]:libera.chat> > <@w[m]:libera.chat> > <@mj-xmr[m]:libera.chat> You also can't tell me that the reason it can't be merged is that "we can't find reviewers". We have a few of them but they can't be demotivated with:... (full message at libera.ems.host/_matrix/media/r0/do…5f6e4f0a8e56ebda6bc8f5aad6a0b4b1d9e)

stay on topic

> <@mj-xmr[m]:libera.chat> > <@w[m]:libera.chat> > <@mj-xmr[m]:libera.chat> You also can't tell me that the reason it can't be merged is that "we can't find reviewers". We have a few of them but they can't be demotivated with:... (full message at libera.ems.host/_matrix/media/r0/do…218085c58f48975fbff5b9939f539d231b4)

Was I referring to you in that message that you quoted ? Why so tin foiled?

What's your GH account w[m] and what are your contributions?

-dev

wrong chat

dev what?

Does anybody know w[m] ?

Or is it safe for me to ignore him?

yes people know me

I'm not asking you

Because he might know something useful.

nobody is or has been ignored by me

But I start to doubt it.

im going to stop now. guy who makes internet threats wants to know who i am

and again

this is -dev

ive got nothing to add so ill shutup now. but calling people sock puppeta and threatening to shoot people.. grow up

nioc: Try to open a CCS Proposal and discover a whole New Realm of jealous sock puppets.

> <@w[m]:libera.chat> im going to stop now. guy who makes internet threats wants to know who i am... (full message at libera.ems.host/_matrix/media/r0/do…ceaea9842d1ad542d015176ba727e40df90)

And don't ping me.

I'm wondering, is ooo still chatting here, or did you guys manage to continue the mudslinging without him ? :D

The water is fine :)

And I can't tell you if ooo is still chatting :)

ooo has yet to read mj's talk about shooting him in -community

w[m]: What? What a paranoid fuck...

Maybe I can buy him some medicine?

Besides shut up. This is -dev

moneromooo: hahaha ooo is off to work on perfecting his code.. probably

You can't make a Mercedes from a pile of shit.

ooo123ooo1234567 I've just read through monero-project/monero #5893 and that whole std::random_device code appeared because "Coverity said so". I don't see why we need something better than rand() in a freaking delay function which is "Sleeping because QUEUE is FULL"

it can probably be removed completely

why random delay there at all

I would prefer it to be removed entirely and have the delay of fixed value (250 ms)

unless there are strong objections

hyc you commented in that old PR, do you have any more context?

Let's move on. 8365 has been approved by 3 reviewers, including me.

(independently of the above discussion)

I doube 8365 is needed at all

there's no need for randomness in that place

aha

unless I don't understand something about that code

it's literally just a delay loop

Try to remove it and run the core tests for a first impression?

that waits until the send queue clears

and then other tests?

good idea

With such code base, there may be many side effects that expect this behavior.

It wasn't English, but YKWIM

Delay will be random anyway, even if you supply fixed "250" value every time

Because Windows, Linux, MacOS and many others are not real-time OS

Yes, but maybe there's a statistical expectation, that say, for 1000 tries there's a result below 10.

one result or 10.

Also 250 is wayyyyyy too high for polling if the queue is empty.

Ah, you're talking about 250ms.

We can still wait a variable length of time to prevent side channel attacks but it wastes an entire quarter of a second just to POLL if the queue is at the right level

Well, it won't be SHORTER than 250, no?

no AFAIK

OK, the way I approach such problems is I try to find which unit tests already cover this piece of code.

I have a script for that for you guys, that's waiting to me reviewed and merged since 1.5 year.

We can wait a whole quarter of a second, but that should only be after we know we can send

rbrunner: ;)

jeffro256[m] 7760 fixes this poll delay by doing async calls

^ It will be useful for anybody who wants to work methodically on the project.

You can use it as it is, no need to merge it.

@sech1 I'll need to look at that specific part of the code, but unless he changed how the chunks are buffered/polled, just making it async won't help unfortunatelhy

making it async frees the current thread so it can do other things

this is how networking code can work efficiently

Right, but it can still starve an indefinite amount of time if it comes back and its still not ready, comes back and still not ready, comes back and still not ready... Even if it's not hogging CPU time

It wouldn't take up CPU time, but I can confirm, that my valgrind tests many times, especially lately, run into a dead loop without any CPU usage. This COULD be this part but could be also another.

Every such jump in total run time was actually manually killed by me.

So I confirm, that the problem exists at least.

not necessarily here though.

mj-xmr[m]: why are you pasting your useless charts everywhere ? is it how competent people are doing at work ?

I'm running "make release-test" without this random delay, we'll seen in a couple of hours

ooo123ooo1234567 why useless charts? It shows that there is a problem somewhere when running with valgrind.

@sech1 The send buffer needs some sort of fairness mechanic

so it is useful, it gives information

<sech1> "it can probably be removed..." <- "I didn't miss, it. I even wrapped that random_device into separate random_delay function in order to remove it completely" yes

useless for the brainless.

Here's the relevant script, just for reference: github.com/monero-project/monero/bl…ster/utils/health/valgrind-tests.sh

sech1: Call at least 1 problem which was fixed with the help of them.

the problem with valgrind

sech1: or noise

which will be fixed

<jeffro256[m]> "Right, but it can still starve..." <- Did you try to read code ? What's the purpose to talk about code without reading it ?

> > <@sech1:libera.chat> so it is useful, it gives information... (full message at libera.ems.host/_matrix/media/r0/do…0ba1eace3e17632726d4778a5689e524078)

> > <@jeffro256:monero.social> Right, but it can still starve an indefinite amount of time if it comes back and its still not ready, comes back and still not ready, comes back and still not ready... Even if it's not hogging CPU time

>

> Did you try to read code ? What's the purpose to talk about code without reading it ?

I specifically said I didn't read that part and that I was going to

It sounds to me like you're wasting time on somebody.

> <@jeffro256:monero.social> > > <@sech1:libera.chat> so it is useful, it gives information... (full message at libera.ems.host/_matrix/media/r0/do…e1258cc8b76abb3c8aeb0c2c4eac4b605b6)

ooo123: are you trying to hinder the progress of Monero as a whole?

Do you have a conflict of interest?

Because it starts to appear so to me.

It was a rhetorical question. I won't see your replies. Just that you know.

<sech1> "ooo123ooo1234567 I've just..." <- The right question: why previous reviewer allowed it ? And it isn't the only example of merged code without proper review

<sech1> "the problem with valgrind" <- yes. If you fix a potential dead loop problem and multiple runs of this valgrind script runs without a hickup, you've pretty much quickly approximated the tedious work of finding the culprit. Leveraging the fact that the script can be automated (because it was written so)

Again not pretty English, but YKWIM.

> > <@sech1:libera.chat> ooo123ooo1234567 I've just read through monero-project/monero #5893 and that whole std::random_device code appeared because "Coverity said so". I don't see why we need something better than rand() in a freaking delay function which is "Sleeping because QUEUE is FULL"

>

> The right question: why previous reviewer allowed it ? And it isn't the only example of merged code without proper review

I agree. This was probably merged with too little review and now we deal with it later. What's your GH handle?

> <@jeffro256:monero.social> > > <@sech1:libera.chat> ooo123ooo1234567 I've just read through monero-project/monero #5893 and that whole std::random_device code appeared because "Coverity said so". I don't see why we need something better than rand() in a freaking delay function which... (full message at libera.ems.host/_matrix/media/r0/do…1c3c9af6899a2df1573fdf15177cb8a33ec)

* There are such example within last 2 months, it isn't fixed problem

What's your GH handle then? Throw a negative review at the problematic ones!

> <@jeffro256:monero.social> > > <@sech1:libera.chat> ooo123ooo1234567 I've just read through monero-project/monero #5893 and that whole std::random_device code appeared because "Coverity said so". I don't see why we need something better than rand() in a freaking delay function which... (full message at libera.ems.host/_matrix/media/r0/do…e461e84d285e3a3ff729e568ddf9cf4359e)

jeffro256[m]: it's "SoCkPuPpEt"

jeffro256[m]: it doesn't matter; focus on problems/solutions

APOLOGIES.

> <@mj-xmr[m]:libera.chat> > <@sech1:libera.chat> the problem with valgrind

>

> yes. If you fix a potential dead loop problem and multiple runs of this valgrind script runs without a hickup, you've pretty much quickly approximated the tedious work of finding the culprit. Leveraging the fact that the script can be automated (because it was written so)

if anyone would read code before approving PR or writing PR then there would be no such problems

not true

bugs exist even in the best reviewed software

I agree.

it's just inevitable once code complexity exceeds certain limit

but bugs per 10k lines of code or other metrics certainly get better if reviews get done properly

Reviews aren't a 100% safeguard of any software. That's why you have to rely on automated systems as well, which, unlike reviewers, don't get tired from having to deal with SoCkPupeTs.

and once again about automated system and code style, why I ask to rename class members to use m_ prefix

buggy code shouldn't even compile ideally

I have ~12+ years of software dev career in teams that prove it. And it wasn't always all my failt!

if you don't use prefixes, you can accidentally use local variable instead of class member in a complex code

and it will compile

just one typo and you use different variable

compiler is your friend and you should help it by writing code that resists bugs by not compiling :)

@w are you on matrix or IRC?

*by giving compile errors

sech1: If you like adventures, you may always set a given clang warning to an error or define your own warning/error in clang, like the one you mentioned.

I'm using the word "adventures" because this useless chart will show you (more or less) how much work you'll have fixing this:

> at the same time you approved monero-project/monero #8365 blindly; facepalm

I didn't "blindly" approve it. I looked at every single line and come to the conclusion that it's better than what we currently have

jeffro256[m]: matrix

ooo123ooo1234567 You keep complaining about the review process, but refuse to participate in it

mj-xmr[m] I always prefer -Wall -Werror where possible

to keep compilation clean

not in Monero codebase unfortunately

> <@jeffro256:monero.social> > at the same time you approved monero-project/monero #8365 blindly; facepalm

>

> I didn't "blindly" approve it. I looked at every single line and come to the conclusion that it's better than what we currently have

random delay isn't needed, but you approve PR that use random generator in a faster way; facepalm

8365 was actually perfectly prepared for a review and Reviewers as human beings. This is a non-verbal show of the contributor's respect to his reviewers.

sech1: We'll get there, if we work as a team.

> <@jeffro256:monero.social> > at the same time you approved monero-project/monero #8365 blindly; facepalm

>

> I didn't "blindly" approve it. I looked at every single line and come to the conclusion that it's better than what we currently have

it's the same as approving unnecessary optimization

* it's the same as approving unnecessary optimization with more code, wider scope (previously it's within 1 function, now it's within 1 class)

Where even such a "useless charlatan" like me can add a bit here or there.

But I don't have to either.

8365 is no unnecessary optimization

it fixes a security issue (system entropy depletion)

Why don't I see any criticism on #8365 ? Let it be known if there's an issue!

whether the randomness is needed there at all, is another topic

> compiler is your friend and you should help it by writing code that resists bugs by not compiling :)

Dang I must be writing some real bug-resistant code on accident :P

If 8365 is an unnecessary optimization, than master MINUS 8365 is worse than that.

> mj-xmr I always prefer -Wall -Werror where possible

-Wall -Wextra -Weffc++ -Werror -pedantic is the way to go

sech1: No, std::random_device may use CSPRNG / /dev/urandom / RDSEED / RDRAND, but in the worst case it will be bottlenecked by CPU rdseed/rdrand instruction, which certainly has higher throughput than rate of accepting network connections

-pedantic is an overkill

> > <@jeffro256:monero.social> > at the same time you approved monero-project/monero #8365 blindly; facepalm... (full message at libera.ems.host/_matrix/media/r0/do…9322620db0e782f5fb055c542f38bf3d7cc)

ooo123ooo1234567 not all systems has rdseed/rdrand

sech1: it isn't security issue

and entropy exhaustion is a real problem

not a security issue?

Imagine you run monerod that exhausts all entropy while syncing

sech1: proof ?

then start monero-wallet-cli and create a new wallet with 0 entropy

understand now?

> <@jeffro256[m]:libera.chat> > mj-xmr I always prefer -Wall -Werror where possible

>

> -Wall -Wextra -Weffc++ -Werror -pedantic is the way to go

Unreal :)

sech1: NO. CARE TO EXPLAIN AGAIN, SLOWLY?

mj-xmr[m]: can you stop with it?

Seconded

+1

Can you say the same to ooo?

let me know how he reacts.

sech1: if it's real problem, then OS is already compromised since user space code can make it vulnerable;

You're not ignored yet, that's why you get asked :)

sech1: proof please

Why thank you :)

it's a problem on uncompromised OS

if your own code exhausts entropy, your new wallet will be predictable

proof ^^^

sech1: can you find link to any successful demo with entropy depletion that way ?

find yourself

Anyway, if it even needs to be said... fixing entropy exhaustion is good. Please merge, etc etc.

I give you ideas, find proofs please if you need it

ooo123ooo123[m] I agree that we should get rid of the random delay eventually, but we don't have to do it in one fell swoop. It's a relatively easy change to review and takes a step in the right direction. Also please, please, please, please put that stuff into the PR reviews. You have some really good insight so document that with a PR review

moneromooo the next question is if it's needed at all (random delay in epee)

so I'd wait with merging 8365

Not only for the author of the PR, but for others who review as well

maybe complete removal of randomness is fine there

Random maybe not, but busy wait is not good.

jeffro256[m]: 1 line changes in wrong direction are also easy to review, but are they needed ?

Random could help with everything waking up at hte same time, but I'm not sure that can happen.

delays are random already

because OS can't guarantee exact delay

> Random maybe not, but busy wait is not good.

that's what I'm saying. It shoudn't be a horribly long wait just to check again to see if it needs to wait another horribly long period of time to check again to see if it ...

jeffro256[m]: I don't enjoy talking with people that write shallow comments

Much more precise than network delays though. A full queue would be due to (if not a bug) network congestion. That'll typically last somewhat longer than a timeslice (I want to say typical 10 ms ?)

"It shoudn't be a horribly long wait just to check again" Maybe an exponential backoff would be better there. 1 ms, 2 ms, 4 ms and so on in the loop. And cap it at 250 ms.

> > <@jeffro256:monero.social> ooo123ooo123[m] I agree that we should get rid of the random delay eventually, but we don't have to do it in one fell swoop. It's a relatively easy change to review and takes a step in the right direction. Also please, please, please, please put that stuff into the PR reviews. You have some really good insight so document that with a PR review

>

> 1 line changes in wrong direction are also easy to review, but are they needed ?

Do you think #8365 is a small step in the wrong direction?

*256 ms

If it's just one thread spamming lots of messages at once, it'll go through faster though, so small-ish delays are probably better, it'll check often but I assume the work is small, so it won't busy spam CPU.

sech1: sech1, that problem require much bigger changes in other parts; it will be just static delay for now; since code is async, it's ok; don't dive into small scope optimizations

Sure, TCP like sounds good to me.

> <@jeffro256:monero.social> > > <@jeffro256:monero.social> ooo123ooo123[m] I agree that we should get rid of the random delay eventually, but we don't have to do it in one fell swoop. It's a relatively easy change to review and takes a step in the right direction. Also please, please, please, please put that... (full message at libera.ems.host/_matrix/media/r0/do…871728a7ce15119343ef272e6862386b6a8)

yes, if it's async then 250 ms delay is not a problem

<ooo123ooo1234567> "Why are you supporting that..." <- What scammer?

I was talking about the current loop that sleeps for 250 ms

sech1: talking with all you here I'm learning more about English, rather than C++

Siren[m]: mj-xmr

Siren[m]: please move to #monero-community

IIRC the only times I saw that queue filling was due to a bug though.

ooo123ooo1234567: Dunno what you're talking about

Oh, and possibly at the end of the process. So keeping the max delay small enough helps a pause at the end. 250 ms sounds... low enough I guess.

moneromooo: This constant doesn't matter; small - problem, big - problem; you're talking about unimportant detail without reading code

Assuming all the threads don't end up sleeping after each other, so 250 ms * 8 would not be nice. I don't know if they can bejave like that off the top of my head.

so checking the stop flag before sleep is probably good.

Like, right before. Might already do.

moneromooo: in PR it's async code, what threads are you talking about ?

moneromooo: "in PR it's async code, what threads are you talking about ?"

from - ooo123ooo1234567:

<sech1> "I give you ideas, find proofs..." <- ideas are easy to create, proof/implementation are not easy, any hint ?

<sech1> "if your own code exhausts..." <- I saw only hardware bug in AMD CPU for which rdseed will return 0x000000, but it's already compromised OS since hardware is broken

or rather letmegooglethat.com/?q=entropy+exhaustion+attack

There are a few P2P threads IIRC. If it doesn't apply, great, nothing to do then.

it's better to remove random delay

too much hassle to get 1 random number between 0 and 49

which doesn't even need to be random

Sure, as long as low enough that a thread doesn't get stuck unnecessarily several times a row, works for me.

core_tests passed without random delay (running tests now)

but if you say that this code is barely executed at all, it doesn't really matter

spitballing: does random delay let it reorder with other jobs if potentially suck in a delay cycle?

Well, I don't remember seeing this happen much in my case. I do remember this code yelling many years ago. Guess it could be the warnings got moved to a lesser log level too for all I know :)

all tests passed without the random delay

sech1: if you PR it, I will start running the valgrind tests already tonight.

Maybe you could put in a log statement in that while loop which prints if the queue is full and then fuzz test it to see if the code even ever triggers

most likely it doesn't

weaveworks/weave #1037#issuecomment-118418860, "Based on that, the only thing we may need to be concerned about is blocking apps on the host that use /dev/random."

linux users are safe even in the worst case when timeout generation from that code will exhaust entropy pool at maximum speed

good

now maybe it's time to remove that code in your PR as there is no reason for it to be there

and PR 8365 is redundant

sech1: look, doing unnecessary change: first reviewer point it and don't dive deeper, not doing unnecessary change: first reviewer point it and don't dive deeper

In both cases if reviewer don't want to spend time then there is no proper review

I'm not sure I will find my own PR for each case, I'm certainly sure there are examples in repo with PRs of others

Also not changing code style: bad, changing code style: why did you do that ?

But all of this doesn't matter, I'm personally will never complain about code style / text / naming / whatever in PRs of others. I will only check it's correct

you only care about correct/incorrect and don't care about future maintainers who can make it incorrect because it has bad style and they don't fully understand it

very short-sighted

OK but this means, that if there's still a reason for a deadlock, it will still be there, but now it will look like a pure CPU hog. We won't be able to distinguish if it's intentional or not.

what CUP hog? I'm only suggesting to change "250 + rng() % 50" to just "250"

sech1: No. This kind of challenge may force others to start care about something hard, instead of focusing on shallow things. But I'm not sure

Ah sorry. Misunderstood.

code style and readability is not shallow things

sech1: It's possible to look at the same thing with different focus / context / angle, and it will look differently. But correct code is working at least

here we fundamentally disagree

correct code can be working, but at the same time be a shit code

from any maintainer's perspective

you can write a single 1000-line function full of spaghetti that is correct and works and even bug free

good like changing anything there

sech1: No, correct code is pretty hard to write

sech1: Code is meant to be read by humans. Machines are happy with binary code.

Also the humans who are meant to read it aren't only the original author.

*good luck

define correct code then

It can be the original author only that is meant to read it, but this doesn't belong to a team/community project.

1000-line spaghetti code function is correct? If it passes all tests and has no bugs?

I just realized that we're selling years of professional experience to somebody who isn't worth it, entirely for free. Good night.

sech1: That's the problem: we are talking without defining common goal explicitly. And implicitly deduced one isn't equal probably

mj-xmr[m]: experience of writing fiction literature in form reports in your case; facepalm

<sech1> "1000-line spaghetti code..." <- I can treat your question as a challenge or blindly agree. I have no idea if there is some edge case where such code would be correct.

<sech1> "1000-line spaghetti code..." <- In most cases spaghetti code is written by people who add small scope short term fixes

define correct

<sech1> "1000-line spaghetti code..." <- Some things with a lot of edge cases can't be implemented without if/else. There is probably even example in one of you rerpo

Things which are easy (within available time) to decompose can be implemented without spaghetti

> <@mj-xmr[m]:libera.chat> > <@sech1:libera.chat> code style and readability is not shallow things... (full message at libera.ems.host/_matrix/media/r0/do…02276ea5c4e4a1299e1bd2d35e9808c7afc)

<mj-xmr[m]> "8365 was actually perfectly..." <- and all reviewers blindly approved; facepalm

<mj-xmr[m]> "8365 was actually perfectly..." <- scammers are very respectful and pleasant, until understand that they will not receive money

<UkoeHB> "spitballing: does random delay..." <- nobody writes async code that requires random delay in order to be correct; only incompetent developers adds random delay in some places in order to pass tests or hide deeper problem; and it was this case : firstly delay, then random delay, then proper usage of random number generator for random delay

<mj-xmr[m]> "Can you say the same to ooo?" <- are you trying to create alternative reality description for people that don't see all msgs ?

mj-xmr[m] ooo123ooo1234567 please keep drama out of -dev to best of your ability. -community may be basically unmoderated (currently), but -dev isn't (supposed to be)

luigi1112: drama is about emotions, it isn't about emotions. There are real problems that are not solved.

luigi1112: And don't put me into the same row with this scammer;

These questions are relevant to anyone who will do the same

the questions do not require inflammatory language

luigi1112: If there would be rules and no blind merges, then It would work

let's improve it

luigi1112: I think I can defend every my word

on a related note, improving things would be a good time to revisit finding a dedicated maintainer

*esp for the monero repo

<mj-xmr[m]> "Reviews aren't a 100% safeguard..." <- I would completely opposite: the most important problems in monero can't be detected by incompetent people no matter what tool they are using; you're clear example;

<sech1> "bugs exist even in the best..." <- It's a comment about hardness to reach perfection, but in current environment I would say even lower boundary of qualification isn't reached yet

<jeffro256[m]> "What's your GH handle then..." <- if you're not anonymous then it doesn't mean that you must ask everyone what is their GH handle. Also GH isn't a measure of anything.

<luigi1112> "the questions do not require..." <- I'll do my best. I just don't like being falsely accused of being a scammer.