<t​zadiko:matrix.org> I've addressed comments on this. If anyone is free, can I get a set of eyes over this PR, please? It's a simply code-cleanup PR, no logic changes.

<t​zadiko:matrix.org> monero-project/monero #9838

<t​zadiko:matrix.org> Going to look through the PRs and review some folk's PRs. If you have one that you find important, please send it my way, I'll prioritise.

what's the tag

18.4?

<s​yntheticbird:monero.social> v18.4.0 yes

done

<s​yntheticbird:monero.social> 🫡

<s​yntheticbird:monero.social> selsta

v0.18.4.0 ( ͡° ͜ʖ ͡°)

too late for me to do GUI, let's do that tag tomorrow

<t​zadiko:matrix.org> What another high priority issue a noob can tackle?

<g​ingeropolous:monero.social> it wasn't the node with the potential hardware issue. thats my seed node. this is the xmrchain node. but yes, yay tag! cause yeah, this coulda been existing for a while, i just run monerod managed by systemd so it just restarts. because internet.

<r​4v3r23:monero.social> release notes?

<o​frnxmr:xmr.mx> Build party first :P

<t​zadiko:matrix.org> Release notes: "Things were changed"

<t​obtoht:monero.social> build party sounds way more fun than verified reproduction

<m​onero.arbo:matrix.org> build seems to have failed somehow, only giving Linux binaries paste.debian.net/1366203

<m​onero.arbo:matrix.org> I'm looking at the build log and I don't see anything that looks particularly weird. end of the build log looks like this (paste.debian.net/1366204), lots of boost errors but the riscv64 build finished, they don't seem relevant

<p​lowsof:matrix.org> why am i seeing a ruby like error that -site had stackoverflow.com/a/75353113

<p​lowsof:matrix.org> from bin/gsign* in Lyza s pastebin

<m​onero.arbo:matrix.org> great question, I thought it seemed weird but was maybe just complaining about missing file. gonna delete stuff and re-try building in the meantime

mine built fine: paste.debian.net/plain/1366210

<o​frnxmr:xmr.mx> Prob have to use docker method bcuz your os is too new

dockrun.sh for me

<m​onero.arbo:matrix.org> that was docker

<o​frnxmr:xmr.mx> dockrun.sh?

<m​onero.arbo:matrix.org> ah no I'm still using this guide github.com/monero-project/monero/bl…/v0.18.4.0/contrib/gitian/README.md

<m​onero.arbo:matrix.org> "For a shortcut using docker follow the instructions in DOCKRUN.md instead of following the rest of this document.."

<m​onero.arbo:matrix.org> BRUH

<m​onero.arbo:matrix.org> maybe we should remove the old docker instructions form this document? :-/

"Gitian host OS should be Ubuntu 18.04 "Bionic Beaver""

<o​frnxmr:xmr.mx> 🤷‍♂️

Well, I tried it on Ubuntu 24.04 and it failed quickly

<o​frnxmr:xmr.mx> had to edit dockrun.sh anyway, since `command -v apt-cache-ng` fails in my sys

Time to spin up my VM, as before

<o​frnxmr:xmr.mx> (i edited it to `command -v apt-cache`)

<m​onero.arbo:matrix.org> bleh I guess I fucked up updating the VM I use for building when guix was added to master

<o​frnxmr:xmr.mx> Dven after installing apt-cache-ng on debian 12, the command doesnt exist. So.. i basically just told the script to check for something that does. Build working np

gitian was successful on CI github.com/monero-project/monero/actions/runs/14140602581

<t​obtoht:monero.social> gitian ci runs will break after github removes the 20.04 runner in two weeks

<t​obtoht:monero.social> i guess we'll have to dockerize that too unless we branch from master soon :D

my host is on 22.04, but it seems weird that it would matter. the container is 18.04.6 LTS. I did not edit dockrun.sh at all. git pull, git submodule update --init --force, git checkout v0.18.4.0, cd contrib/gitian, OPT="-j 12" ./dockrun.sh v0.18.4.0, go to bed, wake up and done

<o​frnxmr:xmr.mx> I'm on debian 12

<o​frnxmr:xmr.mx> `command -v apt-cache-ng` probably stikl works for you on ubuntu 22

<o​frnxmr:xmr.mx> The dockrun script checks if that command works before proceeding to do anything in a container

ah okay, yeah

<o​frnxmr:xmr.mx> monero-project/monero #8105

<o​frnxmr:xmr.mx> relevant

<p​lowsof:matrix.org> good luck dealing with ruby without docker :P

<o​frnxmr:xmr.mx> `sed -i "s/file.exists/file.exist/g" *` 😂

<o​frnxmr:xmr.mx> i only see ruby as dependency for lxc build, but idk

<o​frnxmr:xmr.mx> I used dockrun -> didnt sign -> copied asserts to signing host

<m​onero.arbo:matrix.org> dockrun.sh has def made it past where the old docker directions failed, looks like it will probably finish

I also didn't sign, copied asserts to signing host

<m​onero.arbo:matrix.org> spoke too soon paste.debian.net/1366267 🤷‍♀️

<o​frnxmr:monero.social> Did you use any OPTs?

<m​onero.arbo:matrix.org> ya, OPT="-j 15" ./dockrun.sh v0.18.4.0

<o​frnxmr:monero.social> Reduce the 15 and try again

<o​frnxmr:monero.social> Just a wild guess, bcuz monero will fail to build for me if i set too high of a # on normal builds

<m​onero.arbo:matrix.org> running it with 12 now since that seemed to work for scoob

<o​frnxmr:monero.social> From readme, xmr can use 2gb ram per thread, so ya

I used to do OPT="-j 4 -m 12000" because you ideally want >2G RAM per core, but I added more RAM recently

<m​onero.arbo:matrix.org> my VM has 16 threads and 36 GB of RAM 😤

<m​onero.arbo:matrix.org> > "-j 4 -m 12000"

<m​onero.arbo:matrix.org> ah shit I did -j 15 and it probably defaulted to -m 2000

<t​zadiko:matrix.org> selsta, this is no longer "in progress": monero-project/monero #9831

updated the label

scoobybejesus probably need to drop your merge commit on gitian.sigs pr

oh shoot, that's so annoying.. okay, thanks

I'm reverting and force pushing changes and github is ignoring me.. gonna have to look at it later

<o​frnxmr:xmr.mx> `git reset --hard HEAD~1 ` should drop the merge commit (be careful not to drop your main commit :D)

<t​zadiko:matrix.org> vtnerd: I was looking at TransactionHistory after your comment and see the lock in Refresh and GetAll. I agree that exposing protected state outside of the class is poor design and a bad idea. The change here would be to revert this method to returning by copy, which I don't like. In this copy case, each item in the vector will also be copied, and each item is definitely more than<clipped message>

<t​zadiko:matrix.org> the size of a pointer. I bring up the size of a pointer because the alternative is to just return shared_ptr so only a copy of a pointer will be made - when that is done, clients won't need to change their usage (from -> to .) either. What do you think?

<t​zadiko:matrix.org> I was initially opposed to the shared_ptr idea until I saw the lock and reconciled it with TransactionInfos no longer being pointers.

<t​zadiko:matrix.org> The memory leak / dangling pointer issue exists if I was to revert this to use raw pointers, which I don't want to do

<v​tnerd:monero.social> The lock in `TransactionHistoryImpl` can likely be removed. Every downstream project had to use `TransactionHistory` in a single thread because of the dangling pointers you mentioned

<v​tnerd:monero.social> This allows for `const std::vector<…>&` in that function signature

<v​tnerd:monero.social> whether you remove the pointers from that vector is another topic. removing those pointers is definitely safer than the current design, but requires a copy of every field at construction time.

<v​tnerd:monero.social> one issue is that the design does not allow filtering by subaddress. So you have to get the entire history (from `TransactionHistory`) then iterate over every transaction, and filter on `TransactionInfo::subaddrAccount()`.

<v​tnerd:monero.social> so in the worse case, you’ve just copied tons of data even though only a subset of the history is needed per UI window

<v​tnerd:monero.social> github.com/monero-project/monero-gu…walletqt/TransactionHistory.cpp#L83

<v​tnerd:monero.social> as an example, the GUI currently requests all history, filters on this one field, and then finally copies every field it needs.

<t​zadiko:matrix.org> I can follow up in a PR with a getTransactionBySubAddressAccount Api.

<t​zadiko:matrix.org> Im just concerned a future downstream application will call Refresh and GetAll at the same time, but I guess it's up to them to syncrhonize properly / ensure thread safety.

<t​zadiko:matrix.org> Relaxing that constraint on our end makes sense to me (removing the lock).

<t​zadiko:matrix.org> getTransaction**s**BySubAddressAccount

<v​tnerd:monero.social> adding that function is only addressing a particular symptom and not the general problem. removing the pointers forces the backend to make a copy of every field or use `vector<monero::TransactionInfo>` directly. My lwsf implementation is currently storing things in a `map<public_key, …>` because its quicker for merging the info from the LWS REST API

<v​tnerd:monero.social> or in other words, if another UI wants to filter on another field, another function would have to be added, etc

<v​tnerd:monero.social> luckily the major_index of a subaddress is probably the filter 99% of the time

<t​zadiko:matrix.org> Following up with the PR as it stands (ignoring subaddress for now), Ill remove the locks.