[07:28:41] 622 builds and runs for me. What should I expect when running it with the forked datadir? Should it get on the right chain by reconsidering? Reindexing? I'm trying to think how to positively test with current master that the issue exists, then do the same steps (or tests) on the PR to make sure it doesn't fork. [07:29:01] @salmonskinroll how did you recreate the situation you documented to the PR comments? [07:33:44] I fired up 2 nodes and have them sync up to a certain point and disconnect the two so that they fork, and I would use a node to submit a proposal but then stop staking after a while while the other node keeps staking so it has a longer chain. I'll them connect the two again so that the node that created the proposal will reorg to the other node and therefore the proposal tx would be in an orphan block. @prole [07:34:44] At that point you'll be able to see the "ghost" proposal as my screen shot was showing. [07:35:21] I'll try to test it some more tomorrow and see if it's properly fixed. [08:45:59] Just manually using the devnet? [08:46:16] And connectnode rpc commands in the debug console? [08:47:01] 😎 [10:49:47] @prole that is the same method I used to test on 4.7.1/master [10:49:55] And in that scenario it will fork [10:50:18] Basically the node 1 and node 2 sync to 1500 blocks, then I disconnect the nodes from each other [10:50:32] Then I have node 1 create a payment request [10:50:43] Then shutdown node 1 after 10 blocks [10:50:52] Then I have node 2 create 300 more blocks [10:50:56] Then reconnect both nodes [10:51:14] Node 1 reorgs, to the new chain from node 2 [10:51:18] @prole As you said that the payment block on the forked chain was on a different block number than allowed. I guess it's the same issue like in beginning, just that other nodes, now reject this block what results in a fork. I see here a problem, that nodes sometimes allow payments on a block number where no payments should exist. Is there not a check, thats only allow a payment on the correct block number? [10:52:42] Then node 1 tries to sync with node2, then it forks [10:53:29] @Goku the reason for the fork is because of this check. [10:53:58] @salmonskinroll @aguycalled does my scenario for testing the fork make sense? [10:54:38] I'll do some more testing with this method, I'll let you guys know if I make more progress. [10:54:55] @mxaddict The check of valid payment request and proposals? But not on correct block number right? [10:58:19] Yes, which is why it fails to sync as it's trying to find a block that does not exist on the correct chain [11:00:37] If I understand the issue correctly I think this is what happens: Node 1 has block A with payment request A Node 2 has block B with payment request A Node 2 has more blocks so Node 1 tries to reorg against the blocks in Node 2 Node 1 does syncs blocks from Node 2 Node 1 no longer has block A which in it's copy of payment request A is referred to Node 1 checks block A, it can't find it, so it thinks that Node 2 is lying Node 1 [11:00:38] bans Node 2 FORK 😄 [11:03:20] I'm not 100% sure if this is the correct reason, but it's what I understand from the testing I've done. [11:13:08] Yes! Thats how I understand the issue too. And here I see the problem, that there is no check on the block number. A node should see every block with a payment as invalid. ONLY this one block after a cycle ended (20160) +50, is allowed to contain a payment. But the nodes from the forked chain, show me that there is no such check. My solution is: 1. By default reject payments on all block numbers 2. Only if correct block number is [11:13:09] there, Check if there are valid payment request 3. Payout [11:22:01] @prole @mxaddict @aguycalled [12:21:25] The problem comes up because the payment block number can be variable. Is my view correct? [12:34:09] nope i would say that was not the issue. payouts are rejected if they have already been paid or the payment request is not mature enough. [12:34:25] the issues are at the db level [12:36:23] block 20210 sees a proposal. then that block is disconnected because of a reorg. the proposal should be removed. if the proposal is not correctly removed then it can fail when 20210 comes again [12:40:24] "payouts are rejected if they have already been paid or the payment request is not mature enough" And the other nodes make then a payment on a higher block, what should not be allowed [12:46:45] Should not be a consens on the block high fix it? And if something is wrong with that block it should go back to a certain state before, to read the block again until its correct. [12:48:44] Just wanted to give maybe helpful input 😅 [13:09:12] Because I see a payment on a different block high, as something clearly wrong [13:16:25] it was a design decision to not force nodes to include payouts at a determined height to simplify the logic. independently of that being the best decision or not, i dont see it related [13:47:09] @aguycalled so my testing method is applicable? [14:41:24] yes, only the problem comes in a later block than a and b with a payout referring the payment request [14:42:56] i am thinking a good measure to introduce would be to commit to a hash of the cfunddb state in each block [14:46:58] @mxaddict how do you get the same payment request on node 2 on a different block? [14:47:31] creating the second payment on the second node when they are disconnected [14:48:10] Wouldn't that be a different payment request with different hash then? [14:49:20] yes, you would need to broadcast the same raw transaction. theres a dumpraw parameter on createproposal and createpaymentrequest [14:49:37] then sendrawtransaction hextx [14:49:56] on both nodes [14:50:09] Gotcha gotcha. [14:54:29] Then what mxaddict said makes sense. [15:20:11] I wonder what would happen us the same thing is done to consultations. [15:20:32] the pr needs to be ported to the dao_consultations branch [15:20:38] once approved and confirmed [15:20:46] 👍 [19:25:47] Does the PR include a fix for what mxaddict described? I'll have some time this afternoon to test it [19:54:27] yex [22:08:22] i've opened a pull request to disable creation of proposals and payment requests on mainnet. https://github.com/navcoin/navcoin-core/pull/623 [22:08:53] Just saw that. Be on it soon.