Skip to content

Conversation

@ca333
Copy link

@ca333 ca333 commented Mar 6, 2020

@ca333 ca333 marked this pull request as ready for review March 8, 2020 21:46
@DeckerSU
Copy link

./komodo-test have Segmentation fault, when running testInvalidNotarisationBadOpReturn test:

[ RUN      ] TestEvalNotarisation.testInvalidNotarisationBadOpReturn

Program received signal SIGSEGV, Segmentation fault.
0x00005555557c6cf5 in CheckTxAuthority (tx=..., auth=...) at crosschain_authority.cpp:33
33              if (!eval->GetTxUnconfirmed(txIn.prevout.hash, tx, hashBlock)) return false;
(gdb) bt
#0  0x00005555557c6cf5 in CheckTxAuthority (tx=..., auth=...) at crosschain_authority.cpp:33
#1  0x000055555566a9e0 in Eval::CheckNotaryInputs (this=<optimized out>, tx=..., height=<optimized out>, timestamp=<optimized out>) at cc/eval.cpp:179
#2  0x000055555566d317 in Eval::GetNotarisationData (this=this@entry=0x7fffffffcde0, notaryHash=..., data=...) at cc/eval.cpp:190
#3  0x000055555562338d in TestEvalNotarisation::TestEvalNotarisation_testInvalidNotarisationBadOpReturn_Test::TestBody (this=<optimized out>)
    at test-komodo/test_eval_notarisation.cpp:147
#4  0x0000555555669ecf in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ()
    at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/boost/system/detail/std_interoperability.hpp:142
#5  0x0000555555660d20 in testing::Test::Run() ()
    at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/boost/system/detail/std_interoperability.hpp:142
#6  0x0000555555660ded in testing::TestInfo::Run() ()
    at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/boost/system/detail/std_interoperability.hpp:142
#7  0x0000555555660eeb in testing::TestCase::Run() ()
    at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/boost/system/detail/std_interoperability.hpp:142
#8  0x000055555566117a in testing::internal::UnitTestImpl::RunAllTests() ()
    at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/boost/system/detail/std_interoperability.hpp:142
#9  0x000055555566a387 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::
internal::UnitTestImpl::*)(), char const*) ()
    at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/boost/system/detail/std_interoperability.hpp:142
#10 0x000055555566146c in testing::UnitTest::Run() ()
    at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/boost/system/detail/std_interoperability.hpp:142
#11 0x00005555555eec98 in RUN_ALL_TESTS () at /home/decker/ssd_nvme/komodo_malpill/depends/x86_64-unknown-linux-gnu/share/../include/gtest/gtest.h:2233
#12 main (argc=1, argv=0x7fffffffda38) at test-komodo/main.cpp:21

@tonymorony
Copy link

Btw in case if we starting to work on C code unit-tests base enlarging we can add ./komodo-test tests to Github Action CI/CD.
Please let me know if it neeed @ca333 @DeckerSU

@DeckerSU
Copy link

DeckerSU commented Mar 10, 2020

Btw ... segfault seems happens here:

When we modify hashBlock, right after return it crashes. If we will comment this line and leave hashBlock untouched here, there is no segfault. But coinimport tests will fail:

[  FAILED  ] 8 tests, listed below:
[  FAILED  ] TestCoinImport.testProcessImportThroughPipeline                                                 
[  FAILED  ] TestCoinImport.testImportTombstone
[  FAILED  ] TestCoinImport.testWrongChainId
[  FAILED  ] TestCoinImport.testPayoutTooHigh
[  FAILED  ] TestCoinImport.testAmountInOpret
[  FAILED  ] TestCoinImport.testInvalidPayouts
[  FAILED  ] TestCoinImport.testCouldntLoadMomom
[  FAILED  ] TestCoinImport.testMomomCheckFail

@ca333
Copy link
Author

ca333 commented Mar 10, 2020

./komodo-test have Segmentation fault, when running testInvalidNotarisationBadOpReturn

thanks for hinting @DeckerSU - this one was already known and is not related to the updates from #298 - i can work on a fix at a later stage. But would be glad if you find time earlier though.

@ca333
Copy link
Author

ca333 commented Mar 10, 2020

Btw in case if we starting to work on C code unit-tests base enlarging we can add ./komodo-test tests to Github Action CI/CD.
Please let me know if it neeed @ca333 @DeckerSU

Thanks @tonymorony - yes, lets add that when we fixed komodo-test

@ca333 ca333 requested review from DeckerSU and tonymorony March 10, 2020 11:35
@DeckerSU
Copy link

@ca333 finally, seems i found a root of segfault in komodo-test and fixed it. Apply this patch:

diff --git a/src/test-komodo/test_eval_notarisation.cpp b/src/test-komodo/test_eval_notarisation.cpp
index f0c48f951..2256f4761 100644
--- a/src/test-komodo/test_eval_notarisation.cpp
+++ b/src/test-komodo/test_eval_notarisation.cpp
@@ -130,6 +130,7 @@ TEST(TestEvalNotarisation, testInvalidNotaryPubkey)
 TEST(TestEvalNotarisation, testInvalidNotarisationBadOpReturn)
 {
     EvalMock eval;
+    EVAL_TEST = &eval;
     CMutableTransaction notary(notaryTx);
 
     notary.vout[1].scriptPubKey = CScript() << OP_RETURN << 0;
@@ -143,6 +144,7 @@ TEST(TestEvalNotarisation, testInvalidNotarisationBadOpReturn)
 TEST(TestEvalNotarisation, testInvalidNotarisationTxNotEnoughSigs)
 {
     EvalMock eval;
+    EVAL_TEST = &eval;
     CMutableTransaction notary(notaryTx);
 
     SetupEval(eval, notary, [](CMutableTransaction &tx) {
@@ -157,6 +159,7 @@ TEST(TestEvalNotarisation, testInvalidNotarisationTxNotEnoughSigs)
 TEST(TestEvalNotarisation, testInvalidNotarisationTxDoesntExist)
 {
     EvalMock eval;
+    EVAL_TEST = &eval;
     CMutableTransaction notary(notaryTx);
 
     SetupEval(eval, notary, noop);
@@ -169,6 +172,7 @@ TEST(TestEvalNotarisation, testInvalidNotarisationTxDoesntExist)
 TEST(TestEvalNotarisation, testInvalidNotarisationDupeNotary)
 {
     EvalMock eval;
+    EVAL_TEST = &eval;
     CMutableTransaction notary(notaryTx);
 
     SetupEval(eval, notary, [](CMutableTransaction &tx) {
@@ -183,6 +187,7 @@ TEST(TestEvalNotarisation, testInvalidNotarisationDupeNotary)
 TEST(TestEvalNotarisation, testInvalidNotarisationInputNotCheckSig)
 {
     EvalMock eval;
+    EVAL_TEST = &eval;
     CMutableTransaction notary(notaryTx);
 
     SetupEval(eval, notary, [&](CMutableTransaction &tx) {

as
git apply -v segfault.patch

@ca333
Copy link
Author

ca333 commented Mar 10, 2020

@ca333 finally, seems i found a root of segfault in komodo-test and fixed it.

excellent - thanks @DeckerSU

tonymorony
tonymorony previously approved these changes Mar 11, 2020
Copy link

@tonymorony tonymorony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

komodo-test doesn't crash on my side anymore, just some tests failing as Decker mentioned above
https://paste.ubuntu.com/p/3gq9mdKvPH/

fix rawtransactions tests amount
@DeckerSU
Copy link

DeckerSU commented Mar 11, 2020

DeckerSU/KomodoOcean@2957504 - can we also add analog of this commit in current PR. This will prevent of spamming really big hex strings (in case of splitfund tx expired). I guess so detail log was needed for debug purposes, when we had transtition to overwinter / sapling. But now seems it's don't needed anymore.

p.s. Also, i'm working on a fix for a proper removal of "stucked" txes in mempool. We have a case in which tx can be already in mempool on all nodes, but never will included in block - bcz of komodo_validate_interest violation. Seems we have an error in code, which should remove it. I'm very close to solution, but not sure, will I have time to fix this till this PR will be closed or no.

@DeckerSU
Copy link

Seems i fixed it (need to test). There was a big difference, between how it was:

(ASSETCHAINS_SYMBOL[0] == 0
  && tipindex != 0 && komodo_validate_interest(...) ) < 0

and how it should be:

(ASSETCHAINS_SYMBOL[0] == 0
  && tipindex != 0 && komodo_validate_interest(...) < 0 )

More details here:
DeckerSU/KomodoOcean@1fbf1c4

@ca333 ca333 requested a review from tonymorony March 14, 2020 22:47
@ca333 ca333 merged commit 2cbf4d2 into dev Mar 16, 2020
who-biz pushed a commit to who-biz/komodo that referenced this pull request Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants