.merges

any update on seraphis release date

luigi1112: pls pls pls cli + gui merges

moneroextremist[: far in the future

> restricted-rpc is to avoid fingerprinting and also disable things like start/stop mining and stopping the daemon

selsta I meant that MD5 is not safe for authentication because its so easy reverse an MD5 hash, so any password is not safe and anyone could start/stop mining if they had challenge data and the hash

oh ok, I see

Also how does restricted-rpc prevent fingerprinting?

it removes specific data from get_info that can be used for fingerprinting

Ahh yes I forgot about that good point

selsta do you know of anyone working on TLS for RPC or the levin protcol?

I know vtnerd was doing something called the Noise protocol, which adds a bunch of noise to Levin packets

jeffro256[m]: did you do ./monerod --help and grep for ssl?

there are already options that's why I'm confused

Noise protocol is for P2P traffic and not RPC

Oh wow how did I not know about ssl for rpc

thats cool

i did know noise was for p2p

whoa triple plx

pls

.merges

.merges

Merge queue empty

look at all them merges!

.merge+ 8248 8249

Added

Hey, Management, which channel is for repo.getmonero.org ? I'm getting error: 500

It ran out of disk :S I'll see if I can find what's filled it.

Looks like it's 149 GB of website tarballs for some reason. Not sure if deleting most of them would break it...

All dating from today so I can't even nuke old ones.

There's more space now. If anything breaks, let me know, so I know even if I have nfi how to fix :)

Especially to do with the site repo, since it's the one I nuke stuff in.

It's also a temp solution, since I don't know why these things have been accumulating in the first place...

tarballs for the website should be in their dedicated box IIRC

binaryFate ^

It's tarballs made by gitlab.

I suspect test "builds".

lol, 1.7 million users. Such spam.

Sad gitlab doens't make a one click nuke button for these gobshites.

How is mask normalization handled with BP+? I see where output keys are normalized for comparison against the pseudoOuts yet I don't see where it normalizes the pseudoOuts.

And is that the only consensus change introduced besides the BP format itself? I looked over the diff and just wanted to confirm that was it.

Oddly, it looks like gitlab itself removed those cache files now. I guess it was a temporary "need a lot of disk right now".

Or my removing some stuff made it reset the cache. Either way, there's ample free space again and it seems to still work.

xmrchain node uptime 48 days. just started current master

kayabaNerve: IIRC the pseudoOuts are not normalized. Pre-multiplying the output commitments by 1/8 is a time-saving measure since BP+ wants to multiply them by 8.

... right.

Except they still do the equivalence check

Which means that the inputs, which are either of form 1 or 1/8, are checked against the outputs multiplied by 8 to 1

Doesn't that mean amounts get nuked with only 1/8th being spendable?

hmm good question

... and since that's a multiplicative inverse, doesn't that mean the input amounts become incredibly high and effectively an infinite mint?

I don't know about any normalizing (unless that means reduction ?) but the 1/8 is to make sure the verifier can't ever get anything not in the main subgroup.

I'm referring to the multiplication by cofactor

Because I understand it's done here to ensure the correct subgroup is used but it still effects the b value directly

Well, if you think there might be an exploit, you'd ask luigi1111 (or, if still around somewhere, sarang).

I also understand this never would've made it to prod but I'm pretty sure this is on master with a stagenet height usable for testing?

moneromooo: My comment is the second anyone tries to use this they'll be spending commitments multiplied by INV_EIGHT which isn't feasibly valid in the slightest

I'll rebuild my node later (possibly tomorrow) and try it out. If it immediately breaks then... Yeah. Else I'll try to figure why it isn't breaking and what line I'm missing.

the equivalency check is after multiplying by 8 no?

amount -> *inv8 -> *8 -> check

IIRC rbrunner has this running on a private testnet without issues sending transactions kayabanerve

luigi1112: Right but the issue isn't with the outputs

It's with the inputs

that was my assumption

Because the outputs are saved as *inv8 and not *8 again when used as inputs...

luigi1112: I know saw a mul by cofactor on the outputs

* I only saw a mul by cofactor on the outputs

oh I see

Not to mention if it was on the inputs, we'd have to know which form they were (1 or 1/8)

kayabanerve[m]: maybe you could add a test showing how to exploit that (and then if the test fails, we have a guide for fixing it).

So then the thing is we have to get them from the block as 1/8 yet save them to disk as 1 yet I didn't see code doing that. They looked to be copied but I may have just missed the pointer handling.

Granted, the core tests are a bit of a pita to write...

moneromooo: I do hear you. It's just not an exploit. It's "this shouldn't work even without any trickery, with an honest wallet". Seth For Privacy: suggested it was running on a local net

So I'm more curious what I'm missing then...

OK, I'm not sure what you're talking about exactly, but I know where there's one *8 to match a 1/8, miht be the one you're missing:

luigi1112: github.com/monero-project/monero/bl…/src/ringct/rctSigs.cpp#L1483-L1503

in expand_transaction, in cryptonote_core.cpp IIRC

That's when loading a tx from serialized format.

moneromooo: This would be it if I'm just missing it, thanks :)

kayabanerve[m]: github.com/monero-project/monero/bl…768262/src/ringct/rctSigs.cpp#L1490

Hrm. I remembered wrong actually...

UkoeHB: Right, for outPk

It's a mul by 1/8.

But when that outPk is saved to disk, and eventually loaded as a ring member... It'll be aG + bH * INV_EIGHT

oh hm

Which means that the pseudoOut will need to be for bH INV_EIGHT

Which means you'll be spending a completely disparate amount

seems like that should fail any test param

Which means either it's saving to the disk the *8 form, yet the line you quoted is on a copy, or it has to check every ring member it loads for normalization and repeat *8, voiding any performance savings

luigi1112: Right EXCEPT this will pass BP just fine

this mul 1/8 optimization added so much spaghetti, I don't think it was worthwhile (probably <1% speedup)

It just won't generate a spendable ring member which is why it may have been an oversight

UkoeHB rip it all out

half serious

UkoeHB: This was my stance and why I was here asking about it. AFAICT, it's not correctly implemented, creates disparate block/disk representative when done successfully, and doesn't have testing as of right now which makes the code living on master broken.

is this for the next fork/bp+?

Yeah

Which is why it wasn't noticed. I don't think anyone actually tried spending the BP+ outputs yet :p This would've been caught in a few weeks at worst

wait so I'm understanding: is it proposed that new outputs post fork are stored *inv8, then when loaded *8? But no distinction is made vs pre fork outputs which are obviously not stored that way?

luigi1112: apparently that's what the bp+ pr did

luigi1112: But they're not currently loaded *8 so even if that was the intent...

kayabanerve[m]: .

good work kayabanerve[m] noticing this issue

woof

plz keep investigating :)

You can have it with performance. You just need to save the *8 form and keep INV_EIGHT limited to serialization

But I'm not sure it's feasible to effectively pass it around and I don't recommend the headache

doesn't seem worth at all to me

I think the 1/8 adds too much complication. This is the reason I did not implement 1/8 for key images in seraphis (even though it would be a speedup).

Happy to :) I clicked on it when I reviewed Ukoe's next standing multisig PR (impeccable except for one comment I have yet to hear back on) and it stuck out to me, so figured I'd bring it up

Phew nice find kayabanerve

Surprised that wasn't caught in audits...

I think:

1) see if we can get errors in test to confirm the failure

2) rip it out

UkoeHB: Some dude called Kayaba should probably submit a Ristretto encode and fix this once and for all :p

sethforprivacy: I don't think audits cover the full protocol from that higher vantage point

sethforprivacy: It's an optimization tacked on outside of the BP+ code yet only halfway implemented. It would've immediately created invalid results and broken any testnet it ran on, unless we all are missing where it saves to disk in the proper form (keeping the 1% performance gain)/corrects it on load (worsening performance)

yeah agreed it would fail immediately

(if it exists how we think)

Technically you can still create valid proofs under it.

yeah but naively you wouldn't I would think

luigi1112: want a few million xmr? 👀

probably a lot more than that :P

Well I have to keep some for myself

^_^

this is like the key image thing all over again

But yeah, Monero wallet would've immediately errored

So Ristretto is

1) marginally slower to encode/decode

2) faster to check equality

But I'm still expecting a marginal performance decrease with its introduction. I'm still advocating for it though because it fixes all of this. The end

mul8 when decoding amounts: github.com/monero-project/monero/bl…68262/src/wallet/wallet2.cpp#L11357

(during view scanning)

We just never have this discussion or concern again. Seraphis will already force new keys. We can change curves without issue. We can even promote existing points to Ristretto if we absolutely need to so there truly shouldn't be consensus issues

I've got a local test env of the lastest master including bp+ set up sending and receiving bp+ tx's, spending outputs created from bp

Are we missing an integration test that mines rcttypebpp to chain then tries to spend them out of the ledger?

jberman[m]: In that case we're all missing something and I have no idea where it is. I'm guessing it's saving to disk in mul8 and we just have multiple mul8s

core_tests/bulletproof_plus.cpp ought to do that, but they're really GGGHNNN to write...

Is this possibly the line you're looking for? github.com/monero-project/monero/bl…hain_db/blockchain_db.cpp#L248-L249

this is called right before saving to disk

jberman[m]: did you confirm the tx types produced are bp+?

jberman[m]: that line must be doing all the work, nice find

blockchain_db does perform this

> <@jberman:matrix.org> Is this possibly the line you're looking for? github.com/monero-project/monero/bl…hain_db/blockchain_db.cpp#L248-L249

>

> this is called right before saving to disk

Yep, also just found it

considering how much more difficult it is to reason about the protocol with this change, I'd advocate a PR to undo the 1/8 stuff

... are we keeping this or can we agree

1) this headache is proof it's not worth it

2) if we're already doing this mul 8 multiple times (signature verification, save to disk, AND wallet scanning) we're nuking the performance benefit

3) it's confusing to change the commitment communication format (the one committed to in hashes) midway through the chain with no need to

UkoeHB: Ditto

support

FWIW I don't really care about wallet scanning here, just node verification.

undoing this would require a hard reset of any testnet running this

I thought I was just missing it when I started, but then everyone here also agreed this seemed missing, and yet here it was in blockchain_db .-.

That happened before, not a problem.

the hardfork isn't scheduled in the testnet yet

kayabanerve[m] it did seem too obvious to miss haha

Hey, I coded this, I forget things as a matter of course :/

any volunteers to write the PR? otherwise I can do it (would rather focus on seraphis stuff tbh)

luigi1112: Right. I legitimately thought if it was missing, it would've have to been the BP+ code was the tail result so it was never built off of in the tests :p I thought that may technically be possible?

I'll work on it now. First time doing a PR that matters so will be nice

cool thanks kayabanerve[m]

is the estimate really like 1%?

probably just need to go through the original pr and find all instances

I suppose I should write it. I owe more work to the community and I wrote it... Kinda still in burnout mode though.

moneromooo: I'm excited for the opportunity so don't worry ;)

Thanks kayabanerve[m] :D

It's not every day you get to say you helped decide and implement xmr consensus rules :p

Would be nice to see the chain verification speed change too.

It's replacing 6 doublings with a scalar mult and equality check, if we do actually need to force BP points to be in the main subgroup (which would be implemented the same way the key image check is)

luigi1112: on average, each output in a tx corresponds to ~80-100 variable base multiplications, so avoiding one mul-group-order per output is pretty small

(back of the napkin)

It does remove a exponential to group order, no ? IIRC. Maybe.

To check the thing is in the right subgroup.

yeah, mul-group-order = exponential to group order

Hrm. ISTR this is fairly slow...

in the context of a tx, which has a bp+ proof and clsag proof per output on average, it's not that bad

Alright, fair enough.

IIRC it seemed like a free win...

just making sure I'm not missing something, monero-wallet-rpc's --tx-notify only gets called when the transaction is first seen and when it has one confirm, right?

I was hoping it would call a third time when the funds unlocked, but I'm not seeing that

It's definitely not calling when it reaches 10 confirmations.

For what it's worth, that earlier question whether I am running a testnet: Yes, for weeks already, jberman's view tags branch, to test that. Not BP+ in any case

<moneromooo> "It's definitely not calling when..." <- so to watch for when transactions hit 10 confirms, I need to first add transactions to a group from tx-notify when they hit 1 confirm and then poll them?

Well, poll the height I guess. But yes, polling the txes would also work.

IIRC tippero polls the height.

Jeez. Did I really write that 8 years ago :D

what would polling the height do?

Tell you current height, which you can compare to the height at which the received outputs may be spent.

Not quite, in case of reorg, but close enough I guess.

Ah I see, where do I get the height that they can be spent at?

and is that a better method of checking than watching for 10 confirms?

Good point, they might have an unlock time. In that case, polling the txes is better indeed

ok thanks

unfortunate that polling is required to do this, I was hoping I could avoid it

(polling height was just lightweight)

The monero-javascript library has some nice wallet transaction query methods that may be worth digging into. Not sure what rpc methods they're wrapping: moneroecosystem.org/monero-javascript/MoneroTransferQuery.html

moneromooo: just making sure I'm not missing some better way, tx-notify is what I should be using for making a monero payment processor, right?

thanks, I'll take a look

example query from a project i'm messing with :)... (full message at libera.ems.host/_matrix/media/r0/do…1c47248c57788546fe8a8439fd4eefcb3eb)

Also, in the monero-javascript library, the onOutputReceived event is called 3 times - once when unconfirmed, once when confirmed, and once when unlocked.

cryptogrampy[m]: ah that's definitely useful, I'll look now to see how they do that

rbrunner: could you reapprove the ring size 16 PR? it got rebased

Done.

ty

wow has been running BP+ for 10 months

not sure if what you are discussing was included in that version

nioc: Really interesting to hear. I don't know the full history (the PR is only 4 months old) yet I'd assume it has this optimization. It does, technically, work. It just mutates transaction state vs effective state and redefine commitment formulas which makes it... problematic. It's a hassle not worth it overall.

I did get the presumed patch ready. I've been stuck building for a bit and may go to sleep soon though, so probably looking at publication tomorrow. It's pretty simple overall.

.merge+ 8178

Added

Hello everyone. Can I have a question about a specific issue with monerod in this channel?

If not dev related, ask in #monero:monero.social and ill try to answer you there

w[m]: Ok thank you