--- Log opened Wed Feb 05 00:00:09 2025
00:06 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 252 seconds]
00:33 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
00:56 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 260 seconds]
01:46 -!- gnusha [~gnusha@user/gnusha] has quit [Ping timeout: 244 seconds]
01:46 -!- gnusha [~gnusha@user/gnusha] has joined #bitcoin-core-pr-reviews
01:46 -!- Topic for #bitcoin-core-pr-reviews: https://bitcoincore.reviews | meetings on the first Wednesday of each month at 17:00 UTC
01:46 -!- Topic set by glozow [sid453516@user/glozow] [Sun Jan 28 13:52:40 2024]
01:46 -!- Irssi: #bitcoin-core-pr-reviews: Total of 83 nicks [0 ops, 0 halfops, 0 voices, 83 normal]
01:47 -!- Channel #bitcoin-core-pr-reviews created Wed May 19 12:43:59 2021
01:49 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
01:50 -!- Irssi: Join to #bitcoin-core-pr-reviews was synced in 191 secs
02:21 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 252 seconds]
03:57 -!- theStack [~theStack@95.179.145.232] has quit [Ping timeout: 252 seconds]
04:03 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
04:22 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 276 seconds]
04:24 -!- jaruonic [~jaruonic@2a09:bac2:a688:1046::19f:3e] has quit [Quit: Client closed]
04:29 -!- theStack [~theStack@95.179.145.232] has joined #bitcoin-core-pr-reviews
04:35 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
05:08 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 252 seconds]
05:35 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
05:42 -!- kevkevin [~kevkevin@209.242.39.30] has quit [Ping timeout: 260 seconds]
06:11 -!- jaruonic [~jaruonic@2a09:bac2:a688:1046::19f:3e] has joined #bitcoin-core-pr-reviews
06:16 -!- kevkevin [~kevkevin@209.242.39.30] has joined #bitcoin-core-pr-reviews
07:24 -!- PaperSword1 [~Thunderbi@securemail.qrsnap.io] has joined #bitcoin-core-pr-reviews
07:24 -!- PaperSword [~Thunderbi@securemail.qrsnap.io] has quit [Ping timeout: 246 seconds]
07:24 -!- PaperSword1 is now known as PaperSword
07:37 -!- hernanmarino [~hernanmar@2800:2330:2800:10c:f4b5:f1bd:1c09:c579] has joined #bitcoin-core-pr-reviews
07:38 -!- pyth [~pyth@user/pyth] has quit [Remote host closed the connection]
07:39 -!- pyth [~pyth@user/pyth] has joined #bitcoin-core-pr-reviews
08:17 -!- Guest41 [~Guest85@105.235.132.82] has joined #bitcoin-core-pr-reviews
08:18 -!- Guest41 [~Guest85@105.235.132.82] has quit [Client Quit]
08:25 -!- Polaris [~Polaris@38.207.137.250] has joined #bitcoin-core-pr-reviews
08:35 -!- PaperSword [~Thunderbi@securemail.qrsnap.io] has quit [Quit: PaperSword]
08:43 -!- shesek [~shesek@user/shesek] has quit [Quit: The Lounge - https://thelounge.chat]
08:43 -!- shesek [~shesek@user/shesek] has joined #bitcoin-core-pr-reviews
08:46 -!- monlovesmango [~monlovesm@syn-047-012-107-020.res.spectrum.com] has joined #bitcoin-core-pr-reviews
08:47 -!- Naiyoma [~Naiyoma@154.70.35.218] has joined #bitcoin-core-pr-reviews
08:47 -!- adys [~adys@2a06:c701:cb2e:1500:39b5:7c19:a33f:57b1] has joined #bitcoin-core-pr-reviews
08:52 -!- Naiyoma91 [~Naiyoma@154.70.35.218] has joined #bitcoin-core-pr-reviews
08:54 -!- CosmikDebris [~CosmikDeb@190.210.254.130] has joined #bitcoin-core-pr-reviews
08:54 -!- Polaris [~Polaris@38.207.137.250] has quit [Quit: Client closed]
08:55 -!- Naiyoma [~Naiyoma@154.70.35.218] has quit [Ping timeout: 240 seconds]
08:56 -!- stringintech [~stringint@2602:fa59:6:326::1] has joined #bitcoin-core-pr-reviews
08:57 -!- alfonsoromanz [~alfonsoro@191.85.240.77] has joined #bitcoin-core-pr-reviews
08:58 -!- Guest273 [~Guest273@pool-108-18-150-77.washdc.fios.verizon.net] has joined #bitcoin-core-pr-reviews
08:58 < glozow> hi
08:58 < kevkevin> hi
08:58 < jasan> Hello!
08:59 < instagibbs> hi
08:59 < brunoerg> hi
08:59 < monlovesmango> hello
08:59 < glozow> yay :D
08:59 < CosmikDebris> hi
08:59 < stringintech> Hello
08:59 < marcofleon> hi
08:59 -!- alfonsoromanz [~alfonsoro@191.85.240.77] has quit [Read error: Connection reset by peer]
09:00 < glozow> #startmeeting
09:00 -!- Polaris [~Polaris@38.207.137.250] has joined #bitcoin-core-pr-reviews
09:00 < stickies-v> hi
09:00 < glozow> Welcome to PR review club! Today is TXGRAPH DAY https://bitcoincore.reviews/31363
09:00 -!- alfonsoromanz [~alfonsoro@191.85.240.77] has joined #bitcoin-core-pr-reviews
09:00 < glozow> If you say hi now you'll be in the logs ;)
09:00 -!- Ayelen [~ayelen@191.85.240.77] has joined #bitcoin-core-pr-reviews
09:00 < schmidty> hi
09:00 < marcofleon> hi!!!
09:01 < glozow> Did anybody get a chance to review the PR or look at the notes?
09:01 < kevkevin> hi
09:01 < stringintech> Hi!
09:01 < hernanmarino> Hi ! 
09:01 < alfonsoromanz> hi
09:01 < Polaris> hi
09:01 -!- algo2043 [~algo2043@190.210.254.130] has joined #bitcoin-core-pr-reviews
09:01 < jaruonic> hi
09:01 < Guest273> Yes, quickly went through the notes.
09:01 < kevkevin> I was able to look at the PR briefly to get a general idea of what it is trying to do
09:01 < hernanmarino> just the notes today, it looks interesting 
09:01 < sipa> hi
09:01 < stickies-v> i looked at the notes and linked resources, mostly!
09:01 < marcofleon> notes and a bit of review of the pr
09:01 < instagibbs> read the notes, played with the interface, started seriously looking through the commits today
09:01 < stringintech> Tried to figure out the overall structure through the PR description and header files.
09:02 < glozow> Great! hopefully we can get further today, as ultimately we want to be looking at the code
09:02 < glozow> Does anybody have any general cluster mempool questions before we dive in?
09:03 -!- monlovesmango [~monlovesm@syn-047-012-107-020.res.spectrum.com] has quit [Remote host closed the connection]
09:03 < abubakarsadiq> hi
09:03 < glozow> Ok. Feel free to ask questions whenever, since this is a text meeting
09:03 < glozow> What is the mempool “graph” and to what extent does it exist in the mempool code on master?
09:04 < Murch[m]> Hi
09:04 < marcofleon> in CTxMempoolEntry tx parents and descendants are kept track of... but there's no explicit graph
09:05 < jasan> to know the fees in order to make a heavy block template
09:05 < stringintech> Currently It exists as the ancestor-dependant connections and no such thing as clusters.
09:05 < stringintech> descendant*
09:06 < sipa> marcofleon: well, arguably txgraph doesn't change that... there is still just the set of ancestors and descendants for each transaction there, but it's better encapsulated, and uses more efficient data structures
09:06 < kevkevin> What I understand is its the mempool entries and their parents and descendants 
09:06 < abubakarsadiq> list of all the transactions where connected ones form a single graph where a node in the graph represent a tx
09:06 < stickies-v> the nodes of the graph are the `CTXMemPoolEntry` objects, and the edges are are all their ancestor/dependant connection
09:06 < glozow> Right. When we need to know the ancestors of a transaction, what do we do? Bonus points if you can link me a code example
09:07 < sipa> glozow: in the current master code, i assume you mean?
09:07 < marcofleon> right, that makes sense
09:07 < glozow> yes on master
09:08 < glozow> (will also happily accept an answer that is for calculating the descendant set)
09:09 < stringintech> Have not check out the code but recursively look up parents of all the parents of the tx?
09:09 < abubakarsadiq> you have to recursively find parent of the transactions parents
09:09 < CosmikDebris> https://github.com/bitcoin/bitcoin/blob/a43f08c4ae3235f2fa460dd17a7f5f9f9842efd9/src/txmempool.cpp#L243 invoke `CalculateMemPoolAncestors`
09:09 < stickies-v> recursively use `GetMemPoolParents` and `GetMemPoolChildren`?
09:09 < Guest273>     virtual std::vector<Ref*> GetAncestors(const Ref& arg, bool main_only = false) noexcept = 0; ?
09:10 < sipa> Guest273: that's in the PR; the question is how it works in master today
09:10 < kevkevin> Do we calculate using this function? CTXMemPool::CalculateMemPoolAncestors()
09:10 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
09:10 < marcofleon> CTxMemPool::CalculateMemPoolAncestors gotta be involved
09:10 < glozow> stringintech: abubakarsadiq: stickies-v: yep!
09:10 < marcofleon> haha seems to be a lot
09:10 < glozow> CMPA yep
09:10 < glozow> https://github.com/bitcoin/bitcoin/blob/a43f08c4ae3235f2fa460dd17a7f5f9f9842efd9/src/txmempool.cpp#L172-L200 here's the loop
09:11 < glozow> and descendants: https://github.com/bitcoin/bitcoin/blob/a43f08c4ae3235f2fa460dd17a7f5f9f9842efd9/src/txmempool.cpp#L571-L593
09:11 < glozow> What are the benefits of having a TxGraph, in your own words? Can you think of drawbacks?
09:12 < glozow> These should be pros that we *don't* already have today
09:12 < marcofleon> Just more structured and better encapsulated as sipa said previously. Makes it easier to reason about. Also the atomic operations with staging seems like a good approach. I'm not sure if we do something like that today?
09:12 < stringintech> First thing I noticed when looking at the code was how relatively easy it was to understand things. Even not knowing the previous logic.
09:13 < stringintech> As a first-timer.
09:13 < abubakarsadiq> many benefits, we now group connected transactions into clusters, we maintain the list of this clusters, which make it easy for getting ancestors of transaction, linearization, maintaining cluster size, mining and eviction
09:13 < kevkevin> ya I'd say it makes it easier to reason about the the structure of the graph
09:14 < sipa> as for comparison with *today*, i think the answer is primarily that it brings in all of the cluster mempool goodness: chunk feerates, linearizations, the ability to observe a full ordering of transactions in the mempool
09:14 < instagibbs> marcofleon currently we have no great way of "simualting" entry and testing, so you get weird things in the code in mayn places
09:14 < marcofleon> got it
09:14 < sipa> as for comparison with a theoretical alternative where cluster mempool was brought it, without an intermediary layer that models the transaction graph: encapsulation, and the fact that it's fully specified (almost...) which significantly increases the power of tests for it
09:15 < instagibbs> marcofleon https://github.com/bitcoin/bitcoin/blob/master/src/validation.cpp#L951 f.e.
09:15 < glozow> I really like that it abstracts away all the topological details so e.g. validation isn't trying to figure out how to not double-count replacements and combine package/mempool ancestors etc
09:15 -!- CosmikDebris [~CosmikDeb@190.210.254.130] has quit [Quit: Client closed]
09:15 < glozow> Also it can do that accurately, instead of doing ugly carveouts to try to guess
09:15 -!- CosmikDebris [~CosmikDeb@190.210.254.130] has joined #bitcoin-core-pr-reviews
09:16 < glozow> biggest offender imo https://github.com/bitcoin/bitcoin/blob/a43f08c4ae3235f2fa460dd17a7f5f9f9842efd9/src/validation.cpp#L950-L984
09:16 < marcofleon> The carveouts are indeed ugly
09:17 < glozow> sipa: ooh, making a mental note to ask "which parts are not specified?" later
09:17 < sipa> though to be fair, getting rid of the carveout isn't a benefit of this approach
09:17 < glozow> why not?
09:17 < sipa> it's a choice we make, because it is a policy rule which is incompatible with the approach we want
09:17 < sipa> we could do it without this PR
09:18 < glozow> sipa: you seem to be thinking of cpfp carve out
09:18 < glozow> I'm talking about RBF carve out?
09:18 < sipa> oooh, sorry, i didn't check the link!
09:18 < instagibbs> there are multiple carveouts :(
09:18 < sipa> you are right
09:18 < sipa> i didn't know that was also called carvout
09:19 < instagibbs> very long comment in the code too, gives confidence to quality of design
09:19 < glozow> can anybody think of any drawbacks? :)
09:20 < sipa> we have to do annoying review clubs about it? ;)
09:20 < instagibbs> drawbacks to txgraph?
09:20 < glozow> so, complexity? haha
09:20 < glozow> but encapsulated complexity!
09:20 < sipa> i guess that's the most obvious answer... it's a very non-trivial amount of code that needs review
09:21 < marcofleon> 0 drawbacks for sure... no i guess some overhead with all the additional data structures? but I think the encapsulation and the fact that its much easier to test makes it worth it
09:21 < jasan> what about tx prioritization, is is retained with the cluster look at mempool?
09:22 < jasan> I mean RPC prioritizetransaction
09:22 < sipa> jasan: yes, it is independent, as that lives on a higher level
09:22 < abubakarsadiq> @marcofleon the additional data structures are mostly not drawbacks IIUC some are fixing current issues
09:22 < instagibbs> jasan SetTransactionFee see that
09:22 < monlovesmango> it might me more overhead to just get direct ancestor/child?
09:22 < glozow> jasan: yes, that's what `SetTransactionFee` is for
09:22 < schmidty> jasan: TxGraph::SetTransactionFee()
09:22 < stickies-v> wrt drawbacks: I suppose encapsulation sometimes doesn't allow you to make certain (hacky) performance optimizations without changing the interface?
09:22 < jasan> prioritisetransaction
09:23 < jasan> sipa: thanks
09:23 < abubakarsadiq> like `FeeFrac` it fixes the rounding error of  `CFeeRate`
09:23 < sipa> jasan: in the PR description i state that TxGraph doesn't know about prioritization; that doesn't mean it's incompatible with it - the point is just that it doesn't *care* why the mempool things a particular transaction has a particular fee, but it's perfectly possible for the mempool to apply prioritization before handing fees to txgraph (and will)
09:24 < marcofleon> abubakarsadiq: nice yeah that makes sense. I still need to understand a lot of the lower level pros
09:24 < glozow> the only thing I can think of it is that it's sometimes nice for validation to get to dictate what topology limits are per-tx, e.g. with TRUC and other imagined policies https://delvingbitcoin.org/t/v3-and-some-possible-futures/523
09:25 < sipa> monlovesmango: there is indeed some overhead, though it's tiny (and probably well-compensated by using more efficient data structures internally)... the fact that transaction references always need to be translated from/to TxGraph::Ref* pointers is a runtime cost throughout the design
09:25 < instagibbs> glozow I kind of imagine we'd be able to clean up TRUC logic with txgraph though, even without txgraph driving the logic itself
09:26 < glozow> instagibbs: how?
09:26 < instagibbs> maybe after the hour 👍
09:26 < sipa> glozow: no reason why functionality for testing for particular topologies can't be added to txgraph, but the abstraction does mean there is an extra cost
09:26 < glozow> was thinking about some of the whiteboarding we did on limiting direct parents/children
09:27 < glozow> fosho doable. I'm just trying to come up with something to put on this side of the chart 😂 
09:27 < glozow> Ok let's look at implementation
09:27 < glozow> What is the difference between `TxGraph` and `Cluster`?
09:28 < CosmikDebris> clusters are connected components, the graph is a DAG
09:28 -!- stringintech [~stringint@2602:fa59:6:326::1] has quit [Quit: Client closed]
09:28 < marcofleon> txgraph is the full set of transactions and their relationships. cluster is a group of connected txs within the txgraph
09:28 < monlovesmango> TxGraph holds all transaction refs, clusters are assigned to each transaction
09:28 -!- stringintech [~stringint@2602:fa59:6:326::1] has joined #bitcoin-core-pr-reviews
09:29 < sipa> bonus questions: how many Clusters can an individual transaction be part of, within a TxGraph?
09:29 < monlovesmango> one
09:29 < monlovesmango> ?
09:30 < schmidty> 2
09:30 < sipa> monlovesmango: depends which commit we're looking at, but initially, indeed 1
09:30 < instagibbs> monlovesmango Refs are held by the user of TxGraph, I think "Entry" is the terminlogy?
09:30 < stringintech> maybe 2 considering the staging changes?
09:30 -!- CosmikDebris [~CosmikDeb@190.210.254.130] has quit [Quit: Client closed]
09:30 < sipa> stringintech, schmidty: eventually, indeed 2
09:30 -!- CosmikDebris [~CosmikDeb@190.210.254.130] has joined #bitcoin-core-pr-reviews
09:30 < marcofleon> 2 i think yeah. with staging graph and main graph?
09:31 < abubakarsadiq> 1 to externals, 2 within
09:31 < sipa> abubakarsadiq: well, Cluster is entirely within
09:31 < monlovesmango> if the question is within a single TxGraph it should be 1 right? staging and main are different TxGraphs?
09:31 < sipa> monlovesmango: incorrect, TxGraph encapsulates both the main graph, and optionally a staging graph
09:31 < monlovesmango> ok thank you!
09:32 -!- Polaris [~Polaris@38.207.137.250] has quit [Quit: Client closed]
09:32 < glozow> is it possible for a transaction to never be in a singleton Cluster?
09:32 < sipa> glozow: nice one
09:32 < glozow> (singleton Cluster means there is only 1 tx in the cluster)
09:32 < instagibbs> I don't think so
09:33 < abubakarsadiq> I think no, each transaction has to be added individually first
09:33 < instagibbs> AddTransaction says: "/ Construct a new singleton Cluster (which is necessarily optimally linearized)."
09:33 < stickies-v> aren't operations batched?
09:33 < monlovesmango> how would anchor txs be processed?
09:33 -!- Guest80 [~Guest80@109-186-43-178.bb.netvision.net.il] has joined #bitcoin-core-pr-reviews
09:33 < monlovesmango> *ephemeral anchor txs
09:33 < glozow> stickies-v: good point! let's look at `AddTransaction` https://github.com/bitcoin-core-review-club/bitcoin/blob/3c7732bbbe735d1883cdd1737ec200cab8caa049/src/txgraph.cpp#L1305
09:34 < sipa> stickies-v: they are, but transaction additions are not (only removals and dependency additions are batched)
09:34 < glozow> Looks like we immediately make a cluster for it, so batching doesn't change this
09:34 < glozow> So every entry has to be a singleton at some point
09:34 < instagibbs> monlovesmango TxGraph is unaware of what an anchor is, it just knows there are at least two transactions, one depending on another
09:34 < instagibbs> at given feerates
09:35 < monlovesmango> got it, thanks :)
09:35 < glozow> `TxGraph` doesn't even know what a transaction is
09:35 < glozow> What does it mean for a TxGraph to be oversized? Is that the same as the mempool being full?
09:35 -!- Guest80 [~Guest80@109-186-43-178.bb.netvision.net.il] has quit [Client Quit]
09:35 < CosmikDebris> it means at least one of its clusters exceeds size limits
09:35 < abubakarsadiq> @monlovesmango You add the parent, then add the child, then add the dependency between them
09:36 < marcofleon> its not the same as mempool being full. txgraph being oversized means that at least one cluster exceeds max_cluster_count_limit
09:36 < monlovesmango> it means at least one cluster is exceeding the size limits
09:36 < marcofleon> also txgraph not even knowing what a transaction is is cool
09:36 < glozow> marcofleon: monlovesmango: yep exactly. can be oversized and not full. Can be full and not oversized.
09:37 < abubakarsadiq> unclear to me what next to do txgraph is oversized? what are cant you do at that state
09:38 < instagibbs> monlovesmango size(weight/vbytes) and count (number of txns)
09:38 < glozow> abubakarsadiq: what's the question? are you asking what a client should do if they see that it's oversized?
09:38 < hernanmarino> marcofleon: +1
09:38 < kevkevin> There is a function TxGraphImpl::IsOversized to determine if the txgraph is oversized
09:38 < marcofleon> correct if i'm wrong but I feel Abubakar is asking the. next q in the notes essentially?
09:39 < marcofleon> like what can and can't be done
09:39 < sipa> the interface in txgraph.h should be properly documented with which things you can do while oversized and which you cannot
09:39 < glozow> If a TxGraph is oversized, which functions can still be called, and which ones can’t?
09:40 < schmidty> Functions that query cannot be called, presumably for performance, or?
09:40 < marcofleon> all 4 mutations can be called still
09:40 < CosmikDebris> you can't call stuff that depends on linearizing clusters, like computing chunk feerates
09:40 < sipa> schmidty: no, because oversized clusters internally just cannot be _represented_, so if there are batched operations present which would require an oversized Cluster to be created, they just don't get applied, and instead the TxGraph goes into "oversized" state
09:40 < kevkevin> I see that ApplyDependencies cannot be called if oversized
09:40 < sipa> CosmikDebris: not just linearization!
09:41 < instagibbs> anything that involves larger than O(1)-ish work, I think
09:41 < instagibbs> (can't be)
09:41 < sipa> schmidty: so we solve that by outlawing any operations that could require actually materializing an oversized cluster
09:41 < glozow> to linearize, first you must merge clusters. to merge clusters, you must first ensure you will not be creating a cluster that is too big
09:41 < instagibbs> can't even ask for vector of Refs from cluster
09:41 < stringintech> I had a relevant question. What causes the TxGraph to be not oversized anymore? Only tx removal/evict?
09:41 < schmidty> sipa: thanks!
09:42 < sipa> stringintech: bingo, or if the oversizedness is restricted to a staging graph, AbortStaging() will fix it too (by just throwing it away)
09:42 < glozow> stringintech: yes, you can `RemoveTransaction`. Or you can `AbortStaging` ("I've decided not to accept this tx to mempool")
09:42 < glozow> can `main` be oversized?
09:42 < sipa> instagibbs: i think the threshold is anything that requires O(n^2) work or more (which includes computing the ancestors/descendants of a transaction)
09:43 < sipa> instagibbs: the last PR's TxGraph::Trim function is roughly O(n log n), and can be used in oversized state
09:43 < glozow> Under what circumstances could GetIndividualFeerate and GetChunkFeerate return an empty FeeFrac? Is it possible that one returns an empty FeeFrac and the other doesn’t?
09:44 < stringintech> sipa: glozow: Thanks!
09:44 < instagibbs> sipa fair enough
09:44 < glozow> marcofleon: you mentioned "4 mutations", what are they?
09:45 < monlovesmango> both return empty if tx doesn't exist in txgraph. chunk can return empty when individual does not when oversized
09:45 < marcofleon> AddTransaction, RemoveTransaction, AddDependency, and SetTransactionFee
09:45 < kevkevin> glozow arent they AddTransaction RemoveTransaction AddDepndency and SetTransactionFeeRate?
09:45 -!- Ayelen [~ayelen@191.85.240.77] has quit []
09:46 < sipa> monlovesmango: GetChunkFeerate is just *not allowed* to be called when oversized, so talking about its correctness or what it would return isn't really well-defined
09:46 < monlovesmango> sipa: oh right!
09:47 < glozow> kevkevin: marcofleon: great. and why can we keep calling them when oversized?
09:47 -!- Naiyoma91 [~Naiyoma@154.70.35.218] has quit [Quit: Client closed]
09:48 < marcofleon> could chunkfeerate return empty beacuse MakeAcceptable fails while individualfeerate would be fine?
09:48 < marcofleon> was just trying to answer the second part there but not quite sure
09:48 < sipa> monlovesmango: MakeAcceptable cannot fail
09:48 < glozow> What do you have to do before you know chunk feerate?
09:48 < sipa> eh, marcofleon
09:48 < stringintech> glozow: I guess after the mutations it could be not oversized anymore
09:48 < marcofleon> thanks, need to look at that more
09:49 < sipa> marcofleon: or rather, if it might fail, you're just not allowed to call anything that needs MakeAcceptable (because that implies you're oversized)
09:50 < kevkevin> stringintech: if we're using the AddTransaction mutation would it not still be oversized?
09:50 < sipa> yeah, only RemoveTransaction can result in changing oversized -> not oversized
09:51 < sipa> adding transactions or adding dependencies can only result in going the other way
09:51 < glozow> the answer I was looking for is essentially: txgraph is oversized but also lazy (wow that's kind of mean to say) and none of the mutation functions synchronously do cluster merging or linearization
09:51 < sipa> exactly ^
09:51 < glozow> For example: After calling `TxGraph::RemoveTransaction`, has the transaction been removed from `m_entries`?
09:51 < hernanmarino> glozow:  :))
09:52 < instagibbs> added to m_to_remove
09:53 < stringintech> Earlier glozow: asked if `main` can be oversized too which became my question too :)) I guess it was not answered or I missed the answer.
09:53 < glozow> yep: https://github.com/bitcoin-core-review-club/bitcoin/blob/3c7732bbbe735d1883cdd1737ec200cab8caa049/src/txgraph.cpp#L1327
09:53 < stringintech> kevkevin: right.
09:53 < glozow> what about `AddDependency`, what happens instead of merging clusters? https://github.com/bitcoin-core-review-club/bitcoin/blob/3c7732bbbe735d1883cdd1737ec200cab8caa049/src/txgraph.cpp#L1344
09:53 < sipa> stringintech: what do you think the answer is? can main be oversized?
09:54 < glozow> :)
09:54 < stringintech> As far as I saw in the code (not deep enough though), operations were happening on the upmost clusterset. So if we do not have staging the main could be oversized.
09:55 < marcofleon> Is the tx actually removed when Compact is called?
09:55 < sipa> marcofleon: define "tx actually removed"
09:55 < sipa> the Ref? the Entry? the state of the Locator?
09:56 < marcofleon> hmm yeah... i guess removed from m_entries
09:56 < sipa> marcofleon: right, what happens to the Ref then?
09:57 < glozow> stringintech: yes, it depends on usage. Intended usage is to first stage changes and discard them if they result in an oversized txgraph. But you can also apply things directly to main and make it oversized
09:57 < stringintech> glozow: Thank you
09:57 < glozow> `TxGraph`, at least, does not stop you from doing this
09:58 < kevkevin> I think it can be oversized if we don't have the staging txgraph? since then m_clustersets.size() - 1 = 0
09:58 < kevkevin> the main txgraph
09:58 < marcofleon> The Ref stays i believe...
09:58 < sipa> stringintech: in practice (after follow-up PRs to make the mempool code actually use txgraph), staging will be used for rbf evaluations, while main will be used directly from blocks... and a reorg can add transactions back to the mempool, which could temporarily cause main to be oversized
09:58 < abubakarsadiq> sipa: I think the ref was removed already
09:58 < sipa> abubakarsadiq: bingo
09:58 < marcofleon> oh. Hmm okay
09:59 < glozow> does `CommitStaging` stop you from committing changes that would make `main` oversized?
09:59 < sipa> marcofleon: sorry, it was a trick question (actually, i just forgot that had changed recently), Entrys are not removed from m_entries until the Ref is destructed
09:59 < stringintech> sipa: Hmmm. Thanks!
09:59 < monlovesmango> glozow: no doesn't look like it
09:59 < glozow> https://github.com/bitcoin-core-review-club/bitcoin/blob/3c7732bbbe735d1883cdd1737ec200cab8caa049/src/txgraph.cpp#L1600
09:59 < marcofleon> Got it thanks. Trick questions are how we learn!
09:59 < glozow> Ah we are out of time!
10:00 < jasan> Thank you all!
10:00 -!- algo2043 [~algo2043@190.210.254.130] has quit [Quit: Client closed]
10:00 < kevkevin> Thank you guys!
10:00 < monlovesmango> thank you glozow, sipa, and all!!
10:00 < glozow> Do people want to do another meeting? I am not free at this time tomorrow, but if somebody else wants to run it...?
10:00 < abubakarsadiq> thanks for hosting and the nice docs @glozow
10:00 < stringintech> Thanks for making life easier for first-timers :))
10:00 < sipa> thanks glozow for hosting, and thanks everyone for taking some time to look at my PRs
10:00 < jasan> https://alt.signetfaucet.com/ - TRUC related
10:00 < marcofleon> Thanks glozow and sipa! Good stuff
10:00 -!- alfonsoromanz [~alfonsoro@191.85.240.77] has quit [Remote host closed the connection]
10:00 < monlovesmango> learned a lot :)
10:00 -!- algo2043 [~algo2043@190.210.254.130] has joined #bitcoin-core-pr-reviews
10:00 < hernanmarino> thanks ! 
10:00 < algo2043> thanks!
10:01 < glozow> We could also meet again next week
10:01 < stickies-v> thank you for hosting and your awesome prep work glozow !
10:01 -!- alfonsoromanz [~alfonsoro@191.85.240.77] has joined #bitcoin-core-pr-reviews
10:01 < hernanmarino> next week is better i think 
10:01 < glozow> #endmeeting
10:02 < instagibbs> glozow with txgraph, can't we just enforce "SingleTRUCChecks" since the staged level will have the comprehensive view ot topology?
10:02 -!- theSeattleUnfree [~theSeattl@185.243.57.31] has joined #bitcoin-core-pr-reviews
10:02 < instagibbs> (also make single checks a tiny bit simpler)
10:02 -!- algo2043 [~algo2043@190.210.254.130] has quit [Client Quit]
10:03 -!- CosmikDebris [~CosmikDeb@190.210.254.130] has quit [Quit: Client closed]
10:03 < glozow> instagibbs: you mean tell txgraph to check version?
10:03 < glozow> surely not
10:04 < glozow> Or you mean tell txgraph to check an additional limit for specific transactions?
10:04 < instagibbs> Single checks could grab the cluster of the newest tx, check all txns match version (if one is truc), assert vsizes etc
10:04 < glozow> but this doesn't happen within txgraph, does it?
10:04 < instagibbs> if any truc tx exists, cluster size must be <= 2, and assert vsize 10kvb or 1kvb respectively
10:04 < instagibbs> no, just use txgraph to ask for the staged cluster
10:05 < instagibbs> Prechecks loop adds everything to txgraph, one by one
10:06 < glozow> Oh. by "just enforce" you mean you no longer call the other function?
10:06 -!- alfonsoromanz [~alfonsoro@191.85.240.77] has quit [Ping timeout: 252 seconds]
10:06 < instagibbs> 1) adapt SingleTRUCChecks to just grab the cluster from txgraph and assert goodness
10:06 < instagibbs> 2) delete PackageTRUCChecks
10:07 < glozow> Ok, I was confused at first at how this is any different
10:07 < glozow> Yes you can delete the package one because staged has the package txns
10:07 < instagibbs> it's way less code, and asserting stuff in one place instead of two
10:07 < instagibbs> ok cool 👍
10:08 < glozow> I said this in the notes as well
10:09 < glozow> oh actually forgot to link the post-txgraph version. but yeah `PackageTRUCChecks` is annoying
10:09 < instagibbs> I thought I read the notes 🥲
10:09 < glozow> And also no longer overestimate in `CheckPackageLimits`
10:09 < glozow> "Design and Usage with Existing Mempool"
10:10 < glozow> instagibbs: they are long
10:10 < instagibbs> fair enough, wasn't super obvious what was being said
10:10 < glozow> fair enough
10:11 -!- Talkless [~Talkless@mail.dargis.net] has joined #bitcoin-core-pr-reviews
10:11 < jasan> Do the mempool dumps stay compatible? (e.g. one saved on non-cluster version and loaded on clustered)
10:11 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit []
10:11 < instagibbs>  cutting code in half for truc is nice, ngl
10:12 < sipa> jasan: i think we may later want to change the dump format so it includes linearization order information
10:12 < jasan> sipa: makes sense, thanks
10:12 < instagibbs> hadn't occurred to me, restarting your node may cause different chunks!
10:12 < sipa> yeah
10:13 < sipa> worse ones, but possibly also better ones :)
10:16 < instagibbs> ya feeling lunky, punk?
10:16 < jasan> Please have a look at https://alt.signetfaucet.com/ - It works well even with Blockstream Green wallet (screenshots linked in About). The faucet RBF-replaces the payout each time it get (an async) request. Everything in POSIX shell, uses Bitcoin Core CLI RPCs. Linux here.
10:17 -!- monlovesmango [mon@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
10:17 -!- jaruonic [~jaruonic@2a09:bac2:a688:1046::19f:3e] has quit [Quit: Client closed]
10:17 -!- stringintech [~stringint@2602:fa59:6:326::1] has quit [Quit: Client closed]
10:18 -!- monloves_ [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
10:19 -!- jasan [~j@176.112.177.87] has quit [Quit: Bye!]
10:19 -!- brunoerg [~brunoerg@2804:14d:5285:84b2::1001] has quit [Read error: Connection reset by peer]
10:20 -!- brunoerg [~brunoerg@2804:14d:5285:84b2::1001] has joined #bitcoin-core-pr-reviews
10:26 -!- brunoerg [~brunoerg@2804:14d:5285:84b2::1001] has quit [Read error: Connection reset by peer]
10:26 -!- brunoerg [~brunoerg@2804:14d:5285:84b2::1001] has joined #bitcoin-core-pr-reviews
10:27 -!- Olsen [~Olsen@2804:431:c7cb:91c6:3911:7255:fb05:6d0a] has joined #bitcoin-core-pr-reviews
10:27 -!- Olsen [~Olsen@2804:431:c7cb:91c6:3911:7255:fb05:6d0a] has quit [Client Quit]
10:28 -!- brunoerg [~brunoerg@2804:14d:5285:84b2::1001] has quit [Client Quit]
10:29 -!- Guest8 [~Guest8@91-167-222-204.subs.proxad.net] has joined #bitcoin-core-pr-reviews
10:32 -!- brunoerg [~brunoerg@2804:14d:5285:84b2::1001] has joined #bitcoin-core-pr-reviews
10:33 -!- alfonsoromanz [~alfonsoro@2802:8012:8012:e01:1d4b:dcdd:2f8f:aaa7] has joined #bitcoin-core-pr-reviews
10:35 -!- Guest8 [~Guest8@91-167-222-204.subs.proxad.net] has quit [Quit: Client closed]
10:37 -!- monloves_ [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit [Remote host closed the connection]
10:50 -!- monlovesmango [mon@gateway/vpn/protonvpn/monlovesmango] has quit [Quit: leaving]
10:50 -!- jonatack [~jonatack@user/jonatack] has quit [Read error: Connection reset by peer]
10:51 -!- monlovesmango [mon@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:00 -!- monlovesmango [mon@gateway/vpn/protonvpn/monlovesmango] has quit [Quit: leaving]
11:01 -!- monlovesmango [mon@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:01 -!- monlovesmango [mon@gateway/vpn/protonvpn/monlovesmango] has quit [Client Quit]
11:02 -!- theSeattleUnfree [~theSeattl@185.243.57.31] has quit [Quit: Client closed]
11:12 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:14 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit [Client Quit]
11:14 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:18 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit [Client Quit]
11:18 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:25 -!- grettke [~grettke@syn-184-055-133-000.res.spectrum.com] has joined #bitcoin-core-pr-reviews
11:35 -!- alfonsoromanz [~alfonsoro@2802:8012:8012:e01:1d4b:dcdd:2f8f:aaa7] has quit [Remote host closed the connection]
11:37 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit [Quit: leaving]
11:38 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:39 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit [Client Quit]
11:39 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:41 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit [Client Quit]
11:45 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has joined #bitcoin-core-pr-reviews
11:46 -!- monlovesmango [monlovesma@gateway/vpn/protonvpn/monlovesmango] has quit [Client Quit]
11:49 -!- andrewtoth [~andrewtot@gateway/tor-sasl/andrewtoth] has joined #bitcoin-core-pr-reviews
11:52 -!- Guest273 [~Guest273@pool-108-18-150-77.washdc.fios.verizon.net] has quit [Quit: Client closed]
12:00 -!- Talkless [~Talkless@mail.dargis.net] has quit [Quit: Konversation terminated!]
12:16 -!- Guest97 [~Guest97@94.46.175.132] has joined #bitcoin-core-pr-reviews
12:16 -!- Guest97 [~Guest97@94.46.175.132] has quit [Client Quit]
12:27 -!- adys [~adys@2a06:c701:cb2e:1500:39b5:7c19:a33f:57b1] has quit [Quit: Client closed]