[00:21:26] im getting in bed guys, will catch up in my morning [01:00:47] Yep cool [02:19:05] I've written a test to check that a tx successfully sends after the static reward softfork on your PR branch [02:20:25] do we want to try and construct a malformed block with incorrect stake to make sure that check on line 3322 works [02:20:33] or can we assume that it works as intended [02:23:42] im guessing if that check didnt work it would have been possible to stake a large amount already, so its probably not worth the time. [06:08:40] Are there clear steps to reproduce this bug: https://github.com/NAVCoin/navcoin-core/pull/367 [06:09:05] or is it non-deterministic whether a proposal may revert from expired to pending [06:09:17] @aguycalled @prodpeak [07:40:11] wait, im just testing again, i was editing the test in the wrong file :/ [08:42:15] morning [08:42:27] @prole how is it going? [08:43:31] hey bro [08:44:01] ive added a test to the static-rewards fix which checks that it can send a tx after the soft fork activates [08:44:03] which it can [08:44:14] https://github.com/NAVCoin/navcoin-core/pull/369 [08:44:31] as long as we're happy the stake amount is checked correctly by that other line then im happy with the PR [08:44:57] i then moved on to this one: https://github.com/NAVCoin/navcoin-core/pull/367 [08:45:13] but i tried to run the tests you wrote against master and they passed there also [08:45:25] so i am trying to re-write the tests to reproduce the bug on master [08:45:41] i think im nearly there [08:46:15] the main change is instead of having just one node, ive got 2 nodes running [08:46:27] and i check they both have the same states all the way through the test [08:46:42] then at the end when the proposal is expired, i shut down one node and reindex it [08:46:51] to see if it has a different state than the other node [08:47:13] which is a more accurate reproduction of the bug if i understand the bug correctly [08:47:59] hi craig, steps to reproduce https://github.com/NAVCoin/navcoin-core/pull/367 [08:49:20] https://github.com/NAVCoin/navcoin-core/pull/370 tests are now broken [08:49:50] this is the one Matt's been looking at [08:50:19] he's on his way home from mine now, but he was going to jump back online and post about where he got to with it [08:50:37] he thinks he might have found another bug that needs fixing with it [08:51:49] what's the bug? [08:52:58] i cant quite remember, ill let him explain it when he's back online in a few minutes [08:54:47] so here's the test i wrote [08:54:48] https://github.com/NAVCoin/navcoin-core/pull/372 [08:54:52] it fails against master [08:54:56] at the right place [08:55:11] after i reindex the second node after the proposal has expired [08:55:27] so im going to apply this test to your branch [08:55:45] and double check that your patch makes this test pass [08:56:20] i think this is perhaps the process we should do for bug fixes like this. If a bug is found, start by writing a test which fails correctly on the version with the bug [08:56:47] that way we can explicitly know we have fixed the bug, rather than writing the test after the patch is applied [08:58:05] im not really a big fan of TDD when it comes to regular programming, but i think it makes a lot of sense for patching bugs [09:00:56] id say we need a whole test that the whole state (utxo, cfund, etc) is the same after reindexing [09:01:23] yeah probably [09:01:41] rather than only checking if a concrete state (expired) diverges [09:01:56] okay [09:02:19] should we leave your test as it is then? and write a seperate test that does the reindex [09:02:26] and checks everything matches [09:02:46] i want to run this test against your branch at least [09:02:55] yep [09:11:14] So key change for me is handling the PENDING_VOTING_PREQ state? [09:11:25] No other interface changes? [09:17:01] nope [09:21:30] mmm [09:21:36] my test also fails on your branch alex [09:21:40] not sure if it's just the test [09:21:50] or if the patch isnt fixing the bug [09:22:12] after reindexing, one node says the proposal is expired [09:22:22] the other says its "accepted waiting for the end of the voting period" [09:22:48] i dont have much more energy left in me tonight [09:22:56] but im going to pull this test out into its own file tomorrow [09:23:12] and see what i can uncover [09:24:01] i manually tested it and worked [09:24:10] okay, could just be my test [09:24:14] mc290 said the same [09:24:17] ill double check now [09:24:18] okay cool [09:29:34] if your guys manual testing checks out, then im happy with the fix on that branch [09:29:51] and ill continue to try and write some reindexing tests which check a bunch of community fund states [09:30:00] but consider it non-critical for this update [09:30:19] lets wait to hear what @Matt (Dev) reports about what he found today [09:30:22] you are testing change-logic-count-votes right [09:30:43] yeah i think so [09:30:48] yep [09:31:02] mmm acutally, i think i might not have recompiled after switching branches [09:31:03] hold on [09:31:12] my brain is starting to fade [09:31:59] yeah shit i dont think i recompiled after switching from master to run the test on your branch [09:32:00] derp [09:32:04] let me try again [09:40:26] okay [09:40:31] after recompiling my test passes [09:40:35] so im happy with that fix [09:43:31] ive approved the PR [09:44:05] merged [09:46:06] cool [09:46:19] ill approve the static reward spending fix too [09:46:24] since you're happy with 3322 [09:46:54] but yeah, we should create an issue about testing the POS implementation i think [09:49:56] approved: https://github.com/NAVCoin/navcoin-core/pull/369 [09:51:57] so we just need @Matt (Dev) to come back to check what was the extra bug [09:54:46] yep [09:55:00] and then test the 4.5.1 branch once everything is ready for it [09:55:06] and we should be good to go [09:58:15] im calling it a night. 11pm here [09:58:22] good work though alex, [09:58:23] 😃 [09:58:39] not sure where matt got to [09:59:17] ill be back online tomorrow and ill look at that 3rd PR that matt was testing [09:59:28] and we'll try get this all wrapped up for redistribution by monday [09:59:41] and ill start the notification cycle again [10:06:07] Sorry for being AWOL [10:06:16] Had some things I had to take care of [10:06:29] So what I found is basically what I commented on your PR [10:06:49] https://github.com/NAVCoin/navcoin-core/pull/370/files#diff-7ec3c68a81efff79b6ca22ac1f1eabbaR4067 [10:07:22] We don't release locked funds for proposals that expire with the PENDING_VOTING_PREQ state [10:07:23] @aguycalled [10:08:29] cant see any comment in the pr [10:09:25] Ah I forgot to submit the review [10:09:26] sorry [10:09:30] look again [10:12:33] the test covers if the funds are unlocked afaik [10:12:47] I tested it manually [10:12:50] did you comment this based on looking at the code or did you observe the issue? [10:12:57] I looked at the code first [10:13:06] and then tried to write a test, gave up, tested manually [10:13:24] ok [10:14:17] rough steps: 1. unlock cfund 2. create proposal 3. accept it 4. create payment req 5. let proposal expire 6. check cfund stats and see funds are still locked [10:14:32] # Locked amount should be 0, as this was the only payment request and the proposal was expired assert(self.nodes[0].cfundstats()"funds" == 0) [10:14:38] qa/rpc-tests/cfund-paymentrequest-state-accept-expired-proposal.py [10:16:45] Well then I found another way to make funds locked [10:16:52] because I def managed to do it [10:18:26] im seeing what u mean now [10:20:31] just added ur fix [10:20:37] woo [10:20:45] I haven't tested if it works btw 😅 [10:25:46] but if that fixes the bug I found that branch might be good [10:26:11] I haven't tried checking with reindexing or anything though [10:26:19] but you fix that bug in another branch right [10:31:12] yes [10:35:10] if you could double-check it is fixed now itd be great [10:35:45] I'll do a quick check [10:36:19] 'quick' I think it's doing a full compile lol [10:59:32] i tested manually and works as it should [10:59:53] Sweet [11:00:03] I'm still compiling haha [11:11:16] I'm compiled [11:11:18] will test now [11:31:05] It seems broken for me still [11:31:55] I will clear out my files and remake [11:32:09] I will have to test this tomorrow though [11:49:32] how do you test it [11:54:52] regtest [11:55:01] wait [11:55:05] I will type it out [11:57:24] Boot regtest generate 400 donatefund 10000 generate 1 getnewaddress createproposal addressfromabove 5000 "test" 180 generate 1 voteproposal proposalhash yes generate 250 createpaymentrequest proposalhash 5000 generate 1 # wait at least 3 mins cfundstats listproposals generate 1 cfundstats listproposals [11:57:26] something like that [11:57:58] oh dammit [11:58:05] I deleted the wrong message [11:58:58] sigh [11:59:01] I will type again [12:00:51] # boot regtest generate 400 donatefund 10000 getnewaddress createproposal address 5000 "description" 180 generate 1 voteproposal hash yes generate 250 cfundstats createpaymentrequest hash 5000 "description" generate 1 # wait 3 mins cfundstats listproposals generate 1 cfundstats listproposals [12:00:58] something like that @aguycalled [12:01:32] what is what u see and what do u expect to see? [12:01:51] funds locked = 5000 [12:01:57] I expect it to be 0 [12:02:16] the voting of the payment request is still going [12:02:25] funds will be still locked until the voting is pending [12:02:32] right [12:02:40] well in that case [12:02:48] how can I list the payment request [12:02:52] listproposals expired? [12:02:56] yes [12:03:05] the rpc command listproposals need to be adjusted [12:03:14] mm [12:03:21] ok [12:03:28] well I will retest tomorrow [12:03:34] but sounds like I was just testing wrong [12:05:08] lol I left my charger at craigs [12:05:13] lapto is going to die [12:05:16] so I'm done for tonight [12:12:54] cool [16:01:50] What branch has the PENDING_VOTING_PREQ change? [16:07:28] aguycalled:fix-pr-payout-expired-proposal [16:07:34] Is there an RPC command to get the total Nav in circulation? [16:07:36] Thanks [16:11:29] nope [16:13:14] I ask as I summed all the stake rewards from genesis to best and I do not get the same value as Chainz/CMC [16:13:47] I assume Chainz gets their value from cmc along with the current price and market cap [16:15:59] At block 2,704,002 I count a total of 63,621,566.15158625 Stake rewards. 🤔 [16:19:59] are you counting contributions to the cfund? [16:20:03] the difference can be there [16:26:35] Thanks. That's it! [17:04:02] *** aguycalled_ is now known as aguycalled [18:09:20] from my pov coins the cfund should not be considered in circulation [18:09:25] only when payment requests are paid [19:12:52] Well it's easier for me if that's the case. I'll keep it like it is then. [20:30:22] Approved: - https://github.com/NAVCoin/navcoin-core/pull/371 [20:30:40] @Matt (Dev) @mc290 can you also check [20:33:10] I'm out till the evening [20:33:23] Laptop dead actually haha [20:33:34] Need to get my charger from Craig at some point [21:29:58] I will check a bit later today [21:32:12] i've just upped the merge requirements from 1 PR approval to 2. there's quite a few of us on these now [21:32:18] which is a good thing [21:32:31] ill look at the PR matt's assigned to later today and then also the 4.5.1 branch [21:32:51] im just setting up the new office this morning, finishing the carpet and erecting the desks etc [21:34:25] Do we also want to include these in 4.5.1? [21:34:26] https://github.com/NAVCoin/navcoin-core/pull/374 [21:34:35] https://github.com/NAVCoin/navcoin-core/pull/368 [21:36:58]