<ooo123ooo1234567> "In theory, if p2pool will get..." <- I don't believe such transactions can exist in the mempool. Monero should only allow one.

<MeowingCat> "i would like to implement Monero..." <- There is Python code public which demonstrates libringct binding, if you want to go that low level. If you want to do a wallet, then you want to bind wallet2 though.

ooo123ooo1234567 monerod checks for duplicate key images every time it adds a tx to mempool, so that check is redundant

although insert_key_images allows duplicates if a tx comes from a block (after a reorg)

Hello, since calculating subaddress requires the private spend key, how does the view-only wallet knows which subaddresses to scan for with just the private view key?

By calculating subaddresses, do you mean generating them ?

If so, why do you think this needs the secret spend key ?

the wallet scans 200 (I think) first subaddresses by default

subaddress N is generated from the private view key and N, private spend key is not used

only the public spend key is used

moneromooo can you review the new commits in 8381?

done

<moneromooo> "By calculating subaddresses..." <- Oh, it needs private view key and the primary address. No need private spend key

<sech1> "ooo123ooo1234567 monerod..." <- then remove the same check from fill_block_template

<sech1> "moneromooo can you review the..." <- why did you add that check if it's redundant ?

because it's not redundant

"although insert_key_images allows duplicates if a tx comes from a block (after a reorg)"

so it's not redundant in a rare case when there is a reorg with conflicting transactions

sech1: have you read code to find it ?

yes

sech1: wow

sech1: only that comment or everything related to add/remove txs from mempool ?

comments can be not synchronized with actual code

it's unreliable to rely on comments

the whole insert_key_images code

kei_image_set.insert(id).second || !m_blockchain.txpool_tx_matches_category(id, relay_category::legacy);

this line literally means it's possible to have duplicate key images

if the second term in or clause is true

sech1: why did you not read relevant code before ?

"then remove the same check from fill_block_template" <- why did you?

*why didn't you?

I saw this "if (have_key_images(k_images, tx))" check before

I just disregarded it thinking it was redundant

writing comments about code without reading it in -dev, is it off-topic or not ?

your comments ar 99% offtopic

the only useful comment so far was about these duplicate key images

sech1: I've not noticed that confirmed failure in the next message and suggested to remove "redundant" check from other function too

* noticed that you confirmed failure

<sech1> "the only useful comment so far..." <- why not to use m_txs_by_fee_and_receive_time ? it's already simplified clone of fill_block_template labeled as get_block_template_backlog

I have commented about it in the PR

it's not a clone of fill_block_template

it's "get (hash, weight, fee) for transactions in the pool - the minimum required information to create a block template"

fill_block_template goes to the grocery store _and_ cooks the food

get_block_template_backlog just gives you the shopping list

hmm, in this analogy it's the miner who cooks the food

so fill_block_template just goes to the grocery store

<sech1> "I have commented about it in the..." <- m_txs_by_fee_and_receive_time is up to date, so it can be used

it brings even more copy-pasted code with it

indeed, code without m_txs_by_fee_and_receive_time is cleaner

and faster

and doesn't bring a lot of copy-paste

then why it isn't used everywhere in tx_pool.cpp instead of "m_blockchain.for_all_txpool_txes"?

until I understand this fully, I don't want to change

get_block_template_backlog uses m_blockchain.for_all_txpool_txes because it's next to get_transaction_backlog in the code and follows the same code path

besides, it's irrelevant for this particular PR. This change can't bring more than 1-2% improvement, 99% of CPU time is spent in transaction blob parsing and is_transaction_ready_to_go()

<sech1> "then why it isn't used everywher..." <- everything else is agnostic to order of txs, only mempool picker needs different for the best reward

<sech1> "get_block_template_backlog..." <- get_transaction_backlog is doing everything in one pass and doesn't depend on order of txs

I don't think it really matters here

<sech1> "besides, it's irrelevant for..." <- indeed, it doesn't makes sense to change anything since PR was already approved blindly

sech1: indeed, who cares

you care too much about micro-optimizations

the main purpose of this PR is to reduce the number of is_transaction_ready_to_go calls

which take 99% of time

and it wasn't approved blindly

once again, you waste others time with your useless comments

sech1: certainly not me, problem with key images isn't about micro optimizations and paste.debian.net/hidden/876a7a02 has smaller code

Problem with key images was fixed. Five extra lines of code bother you that much?

<sech1> "the main purpose of this PR is..." <- yes, but it isn't easy task and that PR turned implementation into simplified fill_block_template since it's expected that reader will have more sophisticated solver internally

<sech1> "Problem with key images was..." <- inconsistency between title / pr description / comments and code / actions - yes, any specific change in isolation - no

> <@sech1:libera.chat> the main purpose of this PR is to reduce the number of is_transaction_ready_to_go calls

* yes, but it isn't easy task and that PR turned implementation into simplified fill\_block\_template since it's expected that zmq reader will have more sophisticated solver internally

come on

<sech1> "your comments ar 99% offtopic" <- comments under PR are coming from those who didn't read relevant code. I've spent some time to check all relevant code in p2pool and monero in order to find better way than 8381

<sech1> "once again, you waste others..." <- all 3 comments from that scammer is an example of dumb micro-optimizations which aren't even correct, theoretical comments about architecture are also useless since aren't practical, and the only useful comment about m_txs_by_fee_and_receive_time was ignored by you due to potentially huge copy-paste or lack of synchronization, but i've posted patch that shows how to use it

It wasn't ignored, I wrote at least 3 comments about it :D

I just don't think it improves things enough to be justified

sech1: paste.debian.net/hidden/33dc162d, rebased for key images update

technically, my code sorts transactions better than m_txs_by_fee_and_receive_time

<sech1> "monerod checks every tx before..." <- yes, I checked that notifications with txs contain only uncontroversial txs (double spends aren't allowed there), so only edge case is only in 1st batch received from that function

read the code to find out why

s/only//

sech1: division with double vs integer comparison without division ?

bingo

no rounding errors, more precise fee/byte comparison

<sech1> "technically, my code sorts..." <- can you open separate PR for mempool comparator ?

no, it's not a critical issue

at worst, fill_block_template will miss out on a few piconeros

not worth the time to spend on it

I just pointed it as an example why replacing one with another is not 100% improvement

sech1: not critical -> use m_txs_by_fee_and_receive_time, critical -> use m_txs_by_fee_and_receive_time + separate PR to fix mempool comparator, no third option

1) not critical

2) "use m_txs_by_fee_and_receive_time" - I consider it not critical too

you can open a PR after my PR is merged, if it's so annoying to you. You can also replace other instances of m_blockchain.for_all_txpool_txes while you're at it (if it makes things faster)

and yeah, I'd like to see some numbers, how much faster it is actually

<sech1> "I just pointed it as an example..." <- smaller code + a bit faster vs more precise comparator (how many piconeros would be lost due to greedy cut of mempool instead of global optimizations with all mempool txs)

greedy cut if justified because the miner can start mining faster

he'll get more transactions when they're added to the mempool later

*is justified

sech1: iterating lmdb db vs iterating in-memory set ? at least not worse, at most probably faster

lmdb is memory-mapped too

The actual pointer traversing done in both cases will be similar

sech1: only new txs will be broadcasted via zmq, txs that are in mempool but were discarded by your function will not even reach zmq readers again

I know

that's the tradeoff

sech1: the only missing part is target function, without it it's impossible to choose the best

discarded tx will be sent next time, when more profitable transactions go away

sech1: yes, but optimized reward for parts of mempool may be less than optimized reward for whole mempool

yes, in extreme cases

112.5% is not just a random number

try to find at least one block in Monero blockchain that went above that

i.e. a Monero block bigger than 337.5 Kb

we're talking about such rare cases that it's irrelevant

sech1: it's short-term performance optimization (as everything in p2pool), which will not work in case of growth of txs rate

"not work" is false

it will work, just less than perfect

sech1: I'm sorry, but I'm putting you on /ignore for a while, I'm just sick of seeing half a side of this. If you need something, ask someone else to ping me.

sech1: that PR is needed only in extreme cases (relatively to avg state of things currenly) when mempool is big, it's also extreme case but for some reason it's taken into considerations while others aren't taken

s/considerations/consideration/

<sech1> "that's the tradeoff" <- without common target function there is no way to converge to the same point

<sech1> "you can open a PR after my PR is..." <- It's possible to open PRs in parallel too. Few more questions under your PR and someone can resubmit with title "lost contact with PR author".

hahahaha

* Few more ignored questions under

<sech1> "we're talking about such rare..." <- In theory, someone unbiased could judge what is better, but it needs explicit target function and a judge

<sech1> "I just pointed it as an example..." <- hmm, interesting question: the best comparator for rational numbers

<moneromooo> "sech1: I'm sorry, but I'm..." <- Ah. Thank you for that hint. So good ☺️

<arnuschky[m]> "Ah. Thank you for that hint..." <- I did it too, but in reality it doesn't help to solve problems, only postpone them.

<sech1> "while we're at it, can someone..." <- Btw, that key image issue was unnoticed here monero-project/monero #7891/files, and the same happened in 8381, but you didn't even added comment in PR why it was important to check for duplicate key images

I don't understand why you have to bash down someone for adding a contribution to the project in such a negative non-productive way

we're all humans, we all do mistakes, no one's code is perfect

would be much more productive and less frustrating for all parties if you could just point out the mistakes amicably so one can solve them, rather than putting blame on people

aog: "in such a negative non-productive way" I didn't comment on that PR before reading all relevant code, how is it non-productive ?

aog: searching for mistakes is a work, writing shallow comments - not so much work; see difference ?

the way you communicate and express yourself in non-productive and frustrating for all parties involved, you don't have to put people down to prove a point

aog: I would be satisfied with at least 1-2 people that do checks as deeply I do, everyone else may continue to do whatever they did before

aog: Can call at least 1 opportunity to prove the point in objective way so that no one can escape ?

* Can you call at

I'm sure your deep checks and the attention to detail efforts you put in, are very important for the project and welcomed with open arms; although the way you attack contributors is wrong

prove your point, that's awesome, just try not to be a bitpull looking for blood when you do it if you can

aog: I'm attack incorrect statements and incorrect code and in the worst case people, when there are no other way

s/'m//

I understand, all good

aog: others are balancing their poor code with a lot of comments (in code, in github, in this chat, somewhere else) and it directly interfere with my work here; if these comments are not important -> don't balance poor code with it, if these comments must be correct -> be ready to criticism, but don't force me to participate in this discussion where it's impossible to win/converge to the same point objectively

jesus christ. it's been like 2 days of your nonsense. stop.

dukenukem: point to concrete sentence that you think is nonsense and explain why, i'll reply

ooo123ooo1234567: already pointed out by a handful of people in the last 48 hrs. no need to turn -dev into your own kindergarten bullying camp.

dukenukem: if it's so easy then point out at least 1, otherwise put me on ignore list

no and no. get a mirror, helps with self-awareness.

dukenukem: you're discussing concrete human personality instead of pointing out at least 1 nonsense statement

<dukenukem> "ooo123ooo1234567: already..." <- and try to disconnect your emotions during analysis of msgs, criticism can be treated as bullying if target of discussion isn't clear

it's actually circle: no isolated task -> any criticism to work of someone can be treated as bullying where it's more appropriate -> no way to criticize solution for task

.merges

I tried to make a gitian build of the latest master, and the compiled binary doesn't start on Ubuntu 16.04: paste.debian.net/hidden/7ca5dac4

release v0.17 gitian binaries work

so probably some new or updated external dependency breaks Ubuntu 16.04 compatibility

<sech1> "and yeah, I'd like to see some..." <- paste.debian.net/hidden/f179ff7b, no difference, even "Don't ask for blob in for_all_txpool_txes" + "Avoid excess copying of txblob" aren't visible behind fluctuations