fix: devnet stuck on block 2 with crash

knst commented 3 weeks ago

Issue being fixed or feature implemented

Failure on devnet:

+extra logs haven't got to screenshot:

2024-10-31T13:19:21Z CreateNewBlock(): total size 1000 txs: 0 fees: 0 sigops 100
2024-10-31T13:19:21Z Posix Signal: Aborted

Locally similar error is caught with stacktrace:

    Assertion failure:
      assertion: (tx.nType == T::SPECIALTX_TYPE)
      file: ./evo/specialtx.h, line: 33
      function: std::optional<_Tp> GetTxPayload(const TxType&, bool) [with T = CCbTx; TxType = CTransaction]
       0#: (0x5CE686FB3AB0) stacktraces.cpp:655 - __wrap___assert_fail
       1#: (0x5CE686BB9120) specialtx.h:33      - std::optional<CCbTx> GetTxPayload<CCbTx, CTransaction>(CTransaction const&, bool)
       2#: (0x5CE686BB9120) cbtx.cpp:463        - GetCoinbaseTx(CBlockIndex const*)
       3#: (0x5CE686BB916D) cbtx.cpp:470        - GetNonNullCoinbaseChainlock(CBlockIndex const*)
       4#: (0x5CE686BB92AA) optional:469        - std::_Optional_base_impl<std::pair<CBLSSignature, unsigned int>, std::_Optional_base<std::pair<CBLSSignature, unsigned int>, false, false> >::_M_is_engaged() const
       5#: (0x5CE686BB92AA) optional:986        - std::optional<std::pair<CBLSSignature, unsigned int> >::has_value() const
       6#: (0x5CE686BB92AA) cbtx.cpp:401        - CalcCbTxBestChainlock(llmq::CChainLocksHandler const&, CBlockIndex const*, unsigned int&, CBLSSignature&)
       7#: (0x5CE686D36318) miner.cpp:233       - BlockAssembler::CreateNewBlock(CScript const&)

What was done?

Added check if DIP0003 is indeed activated before retrieving Chainlock of previous block.

How Has This Been Tested?

No crash observed locally

Breaking Changes

N/A

Checklist:

[x] I have performed a self-review of my own code
[x] I have commented my code, particularly in hard-to-understand areas
[x] I have added or updated relevant unit/integration/functional/e2e tests
[x] I have made corresponding changes to the documentation
[x] I have assigned this pull request to a milestone

knst commented 3 weeks ago

pls drop https://github.com/dashpay/dash/commit/24027f8e0e220d09dce2b1a4b5bfb73b7b1af02c

can you be consistent? https://github.com/dashpay/dash/pull/6336#issuecomment-2429469257 previous review you blocked my PR because you didn't like my negative condition, and here: https://github.com/dashpay/dash/pull/6336#issuecomment-2429444025

DashCoreAutoGuix commented 3 weeks ago

Guix Automation has began to build this PR tagged as v22.0.0-devpr6375.dde1edf3. A new comment will be made when the image is pushed.

UdjinM6 commented 3 weeks ago

pls drop 24027f8

can you be consistent? #6336 (comment) previous review you blocked my PR because you didn't like my negative condition, and here: #6336 (comment)

These are 2 completely different cases.

if (A) {
 do_smth_related_to_A
} else {
 do_smth_else
}

vs

if (not A) {
  bail_out
}

UdjinM6 commented 3 weeks ago

btw If you still want to keep 24027f8e0e220d09dce2b1a4b5bfb73b7b1af02c, you should fix its title - it doesn't match the code rn

knst commented 3 weeks ago

pls drop https://github.com/dashpay/dash/commit/24027f8e0e220d09dce2b1a4b5bfb73b7b1af02c

I created new PR https://github.com/dashpay/dash/pull/6376 I will give later quick view to src/llmq, src/evo code, maybe there's some more functions can be hidden from global namespace also.

DashCoreAutoGuix commented 3 weeks ago

Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v22.0.0-devpr6375.dde1edf3. The image should be on dockerhub soon.

dashpay / dash