.merges

.merges

Any objection to switching testnet to v13 on, say, sunday ?

snipa will merge the clsag PR next, then I don't think there's anything consensus to change.

sounds good

grydz: anything need to be changed with Ledger Monero-side code?

IIRC the Trezor folks expected one Trezor-specific PR on the Monero codebase after the CLSAG merge

OK, we'll wait then.

sarang, I need this patch at least: gist.github.com/grydz/dc34efb7e3cd6f91189bf27c2ce62b0b

(Just updated the gist, I removed the modification for v13)

What about next weekend? Or would that be too late

For us, the later the better...

Still debugging why I get a correct CLSAG sig at the end but can't pass the sanity check

Once CLSAG is merged, I'll let the Trezor team know

One thing which helped me debug this is to set all randomness to 0, then give the same inputs with and without changes, and see where it starts diverging.

So you'd do that once with your code and once with the upstream code.

.merges

Is there an easy way to do that? Or should I tweak functions like skGen()?

That's the easy way.

I suppose you can hack the PRNG in random.c too, but that might zero randomness you'd want... not sure.

Alright, thanks!

Would there be an issue with updating testnet even if some Ledger / Trezor changes are missing?

Once Ledger / Trezor has PRed their fixes they can be merged.

Testnet participants will have to compile from source (or use inofficial bins) anyway.

Depends if they change consensus code. I expect they won't, but you never know, it might be our bug :)

Right but if there is a bug like this we have to rollback testnet anyway?

not sure

If it already switched, yes.

My understanding is the Trezor PR will not alter consensus

I can't speak to Ledger

^ grydz ?

Do you think my patch could be integrated in the PR?

This is just simples fixes.

In that way, we could also use testnet for debugging.

(patch here: gist.github.com/grydz/dc34efb7e3cd6f91189bf27c2ce62b0b)

What is this value it does not encrypt now ?

Also you removed bounds checking. Is that on purpose ?

moneromooo, the value z was not created on the HW, we can't encrypt it.

It's derived from the private view key (which was exported).

If it doesn't leak info, and the bounds checking is restored (unless it was redundant), then the patch looks ok to me.

What bound checks?

On the Monero app?

Writing to buffer_send.

Do you mean sending &this->buffer_recv[offset] instead?

I admit I don't know much of the ledger code, but AFAICT &this->buffer_recv[offset] is unrelated ?

recv is ledger to wallet2, send is wallet2 to ledger, right ?

Yes.

Well, it's not what I meant, and I don't see the relevance of your comment so I might be missing somthing.

I just meant you removed the bounds checking code on the send buffer, and this looks dangerous at first glance.

Bounds chekcing being checking you don't write after the buffer.

It might be that other code already checks this, in which case it'd be fine.

(My comment was not relevant at all I just don't see where I removed a bound check!)

- this->send_secret(z.bytes, offset);

+ memmove(this->buffer_send+offset, z.bytes, 32);

Is this when I replaced send_secret() with memmove?

Ok!

send_secret has:

I see now.

moneromooo: please rebase 6111

.merge+ 6769

Added

selsta: done

Hi, how can I get transaction inputs from monerod rpc call? I am using get_transaction_pool rpc method call but I can not find there inputs, there are no input (previous output) address in response json

hyc: sech1: could you review the "core: fix mining from a block that's not the current top" commit in PR 6111?

will try to get to it soon

looks a bit more involved than I can spare right now

thanks

I did a quick look and it looks ok on the surface, but I don't know if it actually works as intended. I assume it's covered by tests?

It is. The tests are the reason I needed this.

I get this but I might be invoking it incorrectly

That usually means the daemon died or hanged.

mining test passes

it happens every time, any logs that would be useful?

I'll try here first. Maybe the two patches don't agree with each other.

it passed on github ci and also on the mac mini, seems like to only happen on my system

rip daemon

.merge- 6757

Removed

moneromooo: that delay was due to core::update_checkpoints() within core::handle_incoming_block(). After added --disable-dns-checkpoints unknown delay stays within 100ms.

Oh really. I did not remember this being synchronous.

Worth removing, IIRC it's on a timer.

(and on another thread)

Thanks for finding it

Yeah, it would be helpful. I've disabled it at all as a workaround.

What's about reported delay, Are there any places for optimizations?

Verification could be parallel. IIRC atm it's per tx.

So you typically get 2 threads (one per output) to verify rct IIRC.

Nothing else big comes to mind atm.

Oh, switching the blockchain lock to a rw lock.

But that's a big change.

6739 (CSlag) merged.

Thank you moo. :D

ty

There might also be some redundancy in tx checking in txpool.

.merges

Erring on the side of caution.

Snipa: 6600 can be merged too I think

the others are still

not fully clear

6600 needs a rebase, same reason 6739 did. 13's going to have a bunch of lockouts it looks like.

Was just looking it over.

Verification of all inputs/outputs of block transactions can be trated as N indepedent tasks or they can be dependent on each other?

sarang: please rebase #6600

moneromooo: This task to make transaction verification as fast as possible with more threads was postponed due to high complexity or low priority?

Do you expect it should be simple to speed up it with more threads?

Anyone willing to redo 5686? OP seems MIA

If not, I suppose we should close the PR

It depends which parts of txes. Range proofs can be done as parallel as you want (and indeed they currently are). Other things are harder. Some things can only be done in parallel if they only depend on old enough data (ie, you're not trying to verify A and B in parallel when B depends on A).

Faster verification is not postponed, which would imply it is scheduled to be done.

As so many things, it's diminishing returns.

So, at a first approximation, it's now less simple to speed up more than it was, and for less gain.

I guess I can rebase that, seems simple enough.

Is it easy to find an example of v12+ block with inner dependencies between transaction?

There won't be deps within a block. Usually at most 10 blocks away, though you might find just one block away, a wallet does this IIRC.

I don't have an example though.

1064ms has been spent on block 2173586 here. Does this block has inner dependencies between txs or any other obstacle that prevent fully parallel txs verification?

block have *

moneromooo: aren’t 10 blocks a consensus rule now?

Can't recall. Either v12 or v13.

It's v12 now.

ah true but when syncing it will still happen for older blocsks?

I mean for when the 10 block rule gets enforced.

yes

It would be enough for mining to optimize v12+ verifications

Can you describe all known obstacles to verify v12+ txs in parallel?

selsta: rebased, CI in progress

A couple are lock contention (would be fixed by the rw lock I mentioned) and dependencies between txes in a span (doesn't apply when you get txes when mining though).

Also, having to add code to do so, which might not be worth the gains.

One thing that might be a fairly easy win:

When a tx is received, mining gets paused, the tx gets verified, mining resumes.

A very early step in tx verification is PoW verification, which is comparatively lengthy, and not parallelizable.

But it could be done without the blockchain lock, so you can still mine (for not much if the block ends up valid though).

Otherwise I'd have to look at the code for a while to see what can still be parallized. Again, I've done some, and the Pareto principle applies.

I'm about mining with separate application connected via rpc to monerod. Inner miner isn't my target.

Then the miner presumably submits only blocks which meet the right difficulty, so... very seldom, right ?

In which case, you should not care much about parallelism. it'd just take about the same amount of CPU time, just on the same core rather than several.

Is that not the case ?

Oh. I suppose you might be after the latency when your miner finds a block at the same time a tx is verifying.

Very seldom but still annoying I guess.

In those cases, chances are your block is an orphan anyway.

(assuming I'm not missing your point)

The aim is to reduce time between events p2p_received_fluffy_block and block_accepted (job propogation delay).

Ah, so I did miss your point.

It's for your own block. Fair enough. Then you do want parallel indeed...

I suppose you can already skip PoW validatrion, on account of having submitted a block because you found a nonce that matches.

I suppose you could indeed relay the block at once, before calling handle_....

Assuming it's from a block template that same daemon created.

hmm core tests failed for the CLSAG PR

multisig functional tests also failed

I should... avoid making tests...

moneromooo: can you take a look? github.com/monero-project/monero/runs/1038290819 (... -> View raw logs)

gen_bp_tx_valid_2, gen_bp_tx_valid_3, gen_bp_tx_valid_16, gen_bp_txs_valid_2_and_2, gen_bp_txs_valid_2_and_3_and_2_and_4

sounds serious :P

Not really. It's the "doesn't use full reward" change.

moneromoo: Isn't a language with static typing all the testing you need? It tests the pieces fit together!

Jokes aside, it is nice to hear there's not a major problem here. Really keeping an eye on this new hard fork :)