Avoid using raw pointers in GetAllNewerThan results

[UdjinM6]

I believe this fixed some crashes for me in GUI (with Governance tab on).

[github-actions]

This Pull Request may conflict if the Pull Requests below are merged first.

#4321
conflictable files: src/rpc/governance.cpp
#4357
conflictable files: src/interfaces/node.cpp
#4478
conflictable files: src/interfaces/node.cpp
#4532
conflictable files: src/interfaces/node.cpp,src/interfaces/node.h
#4465
conflictable files: src/governance/governance.cpp,src/governance/governance.h
#4491
conflictable files: src/governance/governance.cpp
#4506
conflictable files: src/governance/governance.cpp,src/rpc/governance.cpp
#4629
conflictable files: src/qt/governancelist.cpp,src/qt/governancelist.h
#4512
conflictable files: src/rpc/governance.cpp
#4623
conflictable files: src/interfaces/node.cpp,src/qt/clientmodel.cpp

[UdjinM6]

oh, wow! that's a lot of conflicting PRs for what I thought was a simple change 🙈

[PastaPastaPasta]

Can you convert this to using weak ptr? These really shouldn't be owning

[UdjinM6]

Can you convert this to using weak ptr? These really shouldn't be owning

I'm not sure how to do this without a lot of refactoring in CGovernanceManager. A suggestion (or a post-merge followup) patch is welcome :)

[PastaPastaPasta]

how about smth like 6321798 to avoid ownership? (shared_ptr changes may still be appropriate to prevent dangling, although I don't like it)

This should be okay to simply cache these values as we continually call updateProposalList via a timer, which creates brand new Proposal objects each time.

[PastaPastaPasta]

Also is

dash/src/qt/governancelist.cpp

Line 342 in 6321798

newProposals.emplace_back(new Proposal(pGovObj, proposalModel, nAbsVoteReq));

a mem leak? @UdjinM6 @5tefan

(please explain if I'm missing smth)

[PastaPastaPasta]

I would propose a solution like 8a09264 as opposed to introducing shared_ptr usage here. It basically extends the lifetime of the lock, to the lifetime of the returned vector (whose values should be considered dangling after the lock expires). as opposed to creating some 40 shared_ptrs (relatively expensive) and extending the lifetime of the governance objects themselves, we only extend the time that we were locking the mutex. (although this ReturnWithLock struct maybe should have a different name, and maybe be in a different file)

[PastaPastaPasta]

Any update on this?

[PastaPastaPasta]

re, any update on this?

[PastaPastaPasta]

utACK for squash merge