BIP119: Fix plotting of pending transactions for congestion control rates #1898

pull MozirDmitriy wants to merge 1 commits into bitcoin:master from MozirDmitriy:bip changing 1 files +2 −2

MozirDmitriy commented at 5:08 pm on July 17, 2025: none

Correct the assignment of variables when plotting “Confirmed but Pending Transactions” in the simulation. Now, the 1x rate line uses blocktimes_c1 and unspendable, and the 2x rate line uses blocktimes_c2 and unspendable2, ensuring the graph labels and data match correctly. This improves the accuracy and clarity of the simulation results visualization.
BIP119: Fix plotting of pending transactions for congestion control rates 692d371697
jonatack added the label Bug fix on Jul 17, 2025
jonatack commented at 8:37 pm on July 17, 2025: member

(Updated by my more recent comment below.)
jonatack removed the label Bug fix on Jul 17, 2025
jonatack added the label Proposed BIP modification on Jul 17, 2025
jonatack added the label Pending acceptance on Jul 17, 2025
jonatack commented at 8:50 pm on July 18, 2025: member

@MozirDmitriy thanks for your proposal. Would you please provide the images of the plot before and after this change, to compare with mine below, and explain how it is improved? Thanks!

For me, on master:

and this branch:
in bip-0119/simulation.py:126 in 692d371697
121@@ -122,8 +122,8 @@ def make_patch_spines_invisible(ax): 122 123 124 par1.set_ylabel("Confirmed but Pending Transactions") 125- p4, = par1.plot(blocktimes_c1, unspendable2, "c", label="Congestion Control Pending (2x Rate)") 126- p4, = par1.plot(blocktimes_c2, unspendable, "r", label="Congestion Control Pending (1x Rate)") 127+ p4, = par1.plot(blocktimes_c1, unspendable, "c", label="Congestion Control Pending (1x Rate)") 128+ p4b, = par1.plot(blocktimes_c2, unspendable2, "r", label="Congestion Control Pending (2x Rate)")
murchandamus commented at 3:18 pm on July 19, 2025:

Your comment explains what you did, but could you please further explain why the labels of the two lines should be swapped?

I’m also not sure whether the repetition of p4 in the original was a bug, but the change from p4 to p4b looks like a mistake, as p4b never appears again in this file.

MozirDmitriy commented at 4:11 pm on July 19, 2025:

Your comment explains what you did, but could you please further explain why the labels of the two lines should be swapped?

I’m also not sure whether the repetition of p4 in the original was a bug, but the change from p4 to p4b looks like a mistake, as p4b never appears again in this file.

Now for 1x speed blocktimes_c1 and unspendable are used, and for 2x speed blocktimes_c2 and unspendable2 are used. This matches the simulation logic. Labels now correctly reflect the data being displayed. The second line uses a separate p4b variable, allowing both lines to be accessed if needed.

jonatack commented at 9:47 pm on July 25, 2025:

@MozirDmitriy as seen in the comments above, the display is essentially unchanged apart from swapping the two colors.
murchandamus commented at 3:24 pm on July 19, 2025: contributor

As far as I can tell, the only differences between your two images are that the teal and red lines are slightly smoother and the latter is slightly shifted, but I could use a more thorough explanation what the bug in the original was and how your change improves the graph. Especially, I’d like to better understand why the variable was renamed.
jonatack commented at 9:50 pm on July 25, 2025: member

Thank you for your proposal, but per code review and the resulting images this change appears to only be swapping two colors in the plot and not fixing anything. Closing for now, unless new information corrects this.
jonatack closed this on Jul 25, 2025

Contributors
MozirDmitriy jonatack murchandamus

Labels
Proposed BIP modification Pending acceptance