ain icon indicating copy to clipboard operation
ain copied to clipboard

Redundant block height condition check when throwing min required collateral error

Open kyleleow opened this issue 2 years ago • 2 comments

What happened:

Nothing serious or breaking, this is just a minor code refactoring issue. The block height check at line 3117 should be redundant because of line 3112. https://github.com/DeFiCh/ain/blob/cb659a5e48a59a4fd84ec1a2c23ee2f8de275cd2/src/masternodes/mn_checks.cpp#L3112 https://github.com/DeFiCh/ain/blob/cb659a5e48a59a4fd84ec1a2c23ee2f8de275cd2/src/masternodes/mn_checks.cpp#L3117

What you expected to happen:

Line 3117 to be return Res::Err("At least 50%% of the minimum required collateral must be in DFI or DUSD when taking a loan.");

How to reproduce it (as minimally and precisely as possible):

Found in src/masternodes/mn_checks.cpp

What are your environment parameters:

Anything else we need to know?:

kyleleow avatar May 27 '22 10:05 kyleleow

@kyleleow: Thanks for opening an issue, it is currently awaiting triage.

The triage/accepted label can be added by foundation members by writing /triage accepted in a comment.

Details

I am a bot created to help the DeFiCh developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the DeFiCh/oss-governance-bot repository.

defichain-bot avatar May 27 '22 10:05 defichain-bot

It can be > consensus.FortCanningHillHeight && < consensus.FortCanningRoadHeight. It's not the same fork height. This has been refactored and simplified in v3. https://github.com/DeFiCh/ain/blob/29f4624f5eb01c34ef398714e7fa05cc9ac53e79/src/masternodes/consensus/txvisitor.cpp#L318

Jouzo avatar May 27 '22 11:05 Jouzo