wallet-core icon indicating copy to clipboard operation
wallet-core copied to clipboard

Added support for Nervos blockchain, its native token CKB and SUDT (Simple User Defined Tokens).

Open blockchainsimplified-com opened this issue 2 years ago • 23 comments

Description

We've added support for Nervos blockchain and its native token CKB.

How to test

We've written test cases in tests/Nervos folder. We've also written a sample application to test address generation and transactions on Android (not included in this pull request).

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • [x] Create pull request as draft initially, unless its complete.
  • [x] Add tests to cover changes as needed.
  • [x] Update documentation as needed.
  • [x] If there is a related Issue, mention it in the description.

If you're adding a new blockchain

  • [x] I have read the guidelines for adding a new blockchain.

@blockchainsimplified-com could you please check failed CI? we also have code coverage check

hewigovens avatar Jun 07 '22 14:06 hewigovens

Hi @hewigovens @catenocrypt @Milerius thanks for review comments. We will do modifications in the code accordingly and let you know.

@Milerius could you please take another look ?

hewigovens avatar Jun 21 '22 22:06 hewigovens

Hi @hewigovens yes, your help would be greatly appreciated in writing tests for Kotlin / Swift. Would you please let Matt Quinn know when can we have a call to discuss this? Thanks.

@blockchainsimplified-com can you add me to your fork? I will push to this pr directly

hewigovens avatar Aug 04 '22 23:08 hewigovens

Also can you merge master again? we improved build time for Linux

hewigovens avatar Aug 04 '22 23:08 hewigovens

Thanks! Added you to the forked repo.

Hi @hewigovens @catenocrypt thanks for approving the PR. Would you also merge the PR? It's showing following message to us: Only those with write access to this repository can merge pull requests.

I will add some swift tests later

hewigovens avatar Aug 10 '22 01:08 hewigovens

Hi @hewigovens we don't know why the Linux CI build pipeline is failing. I guess it started failing after improvements in the build process was done recently. Would you please help to fix this issue? Thanks.

@blockchainsimplified-com will fix it today, we changed some namespace and build configs

hewigovens avatar Aug 14 '22 02:08 hewigovens

@blockchainsimplified-com I fixed build error, but this test failed somehow, could you please check? it throws exception when calling txPlan.proto(), it's ok to run this single test but failed when running with others

[ RUN      ] TWAnySignerNervos.PlanAndSign_Native_Simple
libc++abi: terminating with uncaught exception of type std::out_of_range: unordered_map::at: key not found
fish: Job 1, 'build/tests/tests tests $argv' terminated by signal SIGABRT (Abort)
libc++abi: terminating with uncaught exception of type std::out_of_range: unordered_map::at: key not found
Process 59363 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
    frame #0: 0x00000001a95b2d98 libsystem_kernel.dylib`__pthread_kill + 8
libsystem_kernel.dylib`__pthread_kill:
->  0x1a95b2d98 <+8>:  b.lo   0x1a95b2db8               ; <+40>
    0x1a95b2d9c <+12>: pacibsp
    0x1a95b2da0 <+16>: stp    x29, x30, [sp, #-0x10]!
    0x1a95b2da4 <+20>: mov    x29, sp
Target 0: (tests) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT
  * frame #0: 0x00000001a95b2d98 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x00000001a95e7ee0 libsystem_pthread.dylib`pthread_kill + 288
    frame #2: 0x00000001a9522340 libsystem_c.dylib`abort + 168
    frame #3: 0x00000001a95a2b08 libc++abi.dylib`abort_message + 132
    frame #4: 0x00000001a9592938 libc++abi.dylib`demangling_terminate_handler() + 312
    frame #5: 0x00000001a9488330 libobjc.A.dylib`_objc_terminate() + 160
    frame #6: 0x00000001a95a1ea4 libc++abi.dylib`std::__terminate(void (*)()) + 20
    frame #7: 0x00000001a95a1e40 libc++abi.dylib`std::terminate() + 64
    frame #8: 0x0000000100ed67f4 tests`__clang_call_terminate + 12
    frame #9: 0x0000000100a18198 tests`TW::Nervos::Signer::plan(signingInput=0x000000016fdfe910) at Signer.cpp:18:1
    frame #10: 0x0000000100a17cf4 tests`void TW::planTemplate<TW::Nervos::Signer, TW::Nervos::Proto::SigningInput>(dataIn=size=474, dataOut=size=0) at CoinEntry.h:80:26
    frame #11: 0x0000000100a17c88 tests`TW::Nervos::Entry::plan(this=0x00000001014b61b0, coin=TWCoinTypeNervos, dataIn=size=474, dataOut=size=0) const at Entry.cpp:30:5
    frame #12: 0x0000000100c40630 tests`TW::anyCoinPlan(coinType=TWCoinTypeNervos, dataIn=size=474, dataOut=size=0) at Coin.cpp:243:17
    frame #13: 0x0000000100932ab0 tests`::TWAnySignerPlan(data=0x000060000020c060, coin=TWCoinTypeNervos) at TWAnySigner.cpp:33:5
    frame #14: 0x000000010032f158 tests`TW::Nervos::tests::TWAnySignerNervos_PlanAndSign_Native_Simple_Test::TestBody(this=0x0000600000014000) at TWAnySignerTests.cpp:148:5
    frame #15: 0x000000010119830c tests`void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(object=0x0000600000014000, method=20 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00, location="the test body")(), char const*) at gtest.cc:2607:10
    frame #16: 0x000000010115e390 tests`void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(object=0x0000600000014000, method=20 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00, location="the test body")(), char const*) at gtest.cc:2643:14
    frame #17: 0x000000010115e2e0 tests`testing::Test::Run(this=0x0000600000014000) at gtest.cc:2682:5
    frame #18: 0x000000010115f338 tests`testing::TestInfo::Run(this=0x0000000105036dc0) at gtest.cc:2861:11
    frame #19: 0x000000010116043c tests`testing::TestSuite::Run(this=0x0000000105036ca0) at gtest.cc:3015:28
    frame #20: 0x000000010116de6c tests`testing::internal::UnitTestImpl::RunAllTests(this=0x0000000105004190) at gtest.cc:5855:44
    frame #21: 0x000000010119d550 tests`bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000105004190, method=88 da 16 01 01 00 00 00 00 00 00 00 00 00 00 00, location="auxiliary test code (environments or event listeners)")(), char const*) at gtest.cc:2607:10
    frame #22: 0x000000010116d860 tests`bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(object=0x0000000105004190, method=88 da 16 01 01 00 00 00 00 00 00 00 00 00 00 00, location="auxiliary test code (environments or event listeners)")(), char const*) at gtest.cc:2643:14
    frame #23: 0x000000010116d750 tests`testing::UnitTest::Run(this=0x00000001014c5600) at gtest.cc:5438:10
    frame #24: 0x0000000100026f3c tests`RUN_ALL_TESTS() at gtest.h:2490:46
    frame #25: 0x0000000100026e1c tests`main(argc=2, argv=0x000000016fdff478) at main.cpp:25:15
    frame #26: 0x000000010446508c dyld`start + 520

hewigovens avatar Aug 14 '22 12:08 hewigovens

Hi @hewigovens thanks for your efforts. We cloned the repo on a fresh Ubuntu 20 machine and ran bootstrap.sh. All tests ran successfully without any error.

Only place where we are calling function at on std::unordered_map is on constants defined in the file Constants.h viz. gHashTypeRegistry and gDepTypeRegistry. These are static constants and might not be initialized properly due to recent optimizations. Do you think this would be the case?

@blockchainsimplified-com that might be two problems here:

  1. using unordered_map for type registry doesn't make much sense, only 2~3 cases, just do if or switch
  2. the crash is on macOS without unity build, highly chance it will also occur on iOS; I also tried Linux, it worked

I dumped the error value, you can see the hashType is a random value 895770982

call type.proto()
hashType is 895770982
Signer::plan() errorunordered_map::at: key not found

hewigovens avatar Aug 16 '22 23:08 hewigovens

Hi @hewigovens we used unordered_map as per this review comment.

We've remove unordered_map and instead used a normal string array with for loop and if(...) condition.

The error showing up when we run multiple test cases might be because at several places, we did std::move as per this review comment. Let us try to reproduce this error on our Mac setup and try to figure out which std::move is causing this. We will get back to you soon.

Hi @hewigovens we have pushed one commit. Would you approve workflows and see if this resolves CI issues? Thanks.

@blockchainsimplified-com

[ RUN      ] TWAnySignerNervos.PlanAndSign_Native_Simple
AddressSanitizer:DEADLYSIGNAL
=================================================================
==5154==ERROR: AddressSanitizer: SEGV on unknown address 0x558ea287bbf0 (pc 0x5588a9bbcac8 bp 0x7ffcfe9b10f0 sp 0x7ffcfe9b0de0 T0)
==5154==The signal is caused by a READ memory access.
    #0 0x5588a9bbcac8 in void TW::Nervos::Proto::Script::set_hash_type<char const*&>(char const*&) /home/runner/work/wallet-core/wallet-core/src/proto/Nervos.pb.h:3624:106
    #1 0x5588a9bbcac8 in TW::Nervos::Script::proto() const /home/runner/work/wallet-core/wallet-core/src/Nervos/Script.h:100:16
    #2 0x5588aa8f952a in TW::Nervos::Cell::proto() const /home/runner/work/wallet-core/wallet-core/src/Nervos/Cell.h:94:37
    #3 0x5588aa8f0e10 in TW::Nervos::TransactionPlan::proto() const /home/runner/work/wallet-core/wallet-core/src/Nervos/TransactionPlan.h:87:49
    #4 0x5588aa8d8b05 in TW::Nervos::Signer::plan(TW::Nervos::Proto::SigningInput const&) /home/runner/work/wallet-core/wallet-core/src/Nervos/Signer.cpp:17:19
    #5 0x5588aa8efead in void TW::planTemplate<TW::Nervos::Signer, TW::Nervos::Proto::SigningInput>(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> >&) /home/runner/work/wallet-core/wallet-core/src/CoinEntry.h:80:26
    #6 0x5588aa8d8205 in TW::Nervos::Entry::plan(TWCoinType, std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> >&) const /home/runner/work/wallet-core/wallet-core/src/Nervos/Entry.cpp:30:5
    #7 0x5588aacc4ca0 in TW::anyCoinPlan(TWCoinType, std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> >&) /home/runner/work/wallet-core/wallet-core/src/Coin.cpp:243:17
    #8 0x5588aa7233aa in TWAnySignerPlan /home/runner/work/wallet-core/wallet-core/src/interface/TWAnySigner.cpp:33:5
    #9 0x5588a9b8fe63 in TW::Nervos::tests::TWAnySignerNervos_PlanAndSign_Native_Simple_Test::TestBody() /home/runner/work/wallet-core/wallet-core/tests/Nervos/TWAnySignerTests.cpp:144:5
    #10 0x5588ab4ce28a in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:2607:10
    #11 0x5588ab4b83b9 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:2643:14
    #12 0x5588ab493b62 in testing::Test::Run() /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:2682:5
    #13 0x5588ab4948c8 in testing::TestInfo::Run() /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:2861:11
    #14 0x5588ab4950e3 in testing::TestSuite::Run() /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:3015:28
    #15 0x5588ab4a5a61 in testing::internal::UnitTestImpl::RunAllTests() /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:5855:44
    #16 0x5588ab4d093a in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:2607:10
    #17 0x5588ab4ba879 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:2643:14
    #18 0x5588ab4a55ca in testing::UnitTest::Run() /home/runner/work/wallet-core/wallet-core/build/local/src/gtest/googletest-release-1.11.0/googletest/src/gtest.cc:5438:10
    #19 0x5588a956a4e2 in RUN_ALL_TESTS() /home/runner/work/wallet-core/wallet-core/build/local/include/gtest/gtest.h:2490:46
    #20 0x5588a9548d20 in main /home/runner/work/wallet-core/wallet-core/tests/main.cpp:25:15
    #21 0x7f21cf91dd8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
    #22 0x7f21cf91de3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 69389d485a9793dbe873f0ea2c93e02efaa9aa3d)
    #23 0x5588a9435594 in _start (/home/runner/work/wallet-core/wallet-core/build/tests/tests+0x19c6594) (BuildId: a3598bcbe0278d12d13662b4079236850ddd98ca)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/runner/work/wallet-core/wallet-core/src/proto/Nervos.pb.h:3624:106 in void TW::Nervos::Proto::Script::set_hash_type<char const*&>(char const*&)
==5154==ABORTING

hewigovens avatar Aug 18 '22 22:08 hewigovens

Hi @hewigovens we did similar fix at one more place. This should fix the problem now.

@blockchainsimplified-com nice, CI is running, I see two warnings

/Users/hewig/workspace/trust/wallet-core/src/Nervos/Address.cpp:35:61: warning: declaration shadows a field of 'TW::Nervos::Address' [-Wshadow]
bool Address::decode(const std::string& string, const char* hrp) noexcept {
                                                            ^
/Users/hewig/workspace/trust/wallet-core/src/Nervos/Address.h:37:17: note: previous declaration is here
    const char* hrp;
                ^
1 warning generated.
[78/372] Building CXX object CMakeFiles/TrustWalletCore.dir/src/Nervos/Transaction.cpp.o
/Users/hewig/workspace/trust/wallet-core/src/Nervos/Transaction.cpp:193:87: warning: declaration shadows a field of 'TW::Nervos::Transaction' [-Wshadow]
                                                       const Data& txHash, Witnesses& witnesses) {
                                                                                      ^
/Users/hewig/workspace/trust/wallet-core/src/Nervos/Transaction.h:53:23: note: previous declaration is here
    std::vector<Data> witnesses;
                      ^
1 warning generated.
[372/372] Linking CXX executable tests/tests

hewigovens avatar Aug 19 '22 02:08 hewigovens

@hewigovens thanks. Those warnings are because parameter name and member variable name are same. For example hrp:

bool Address::decode(const std::string& string, const char* hrp) noexcept {
    this->hrp = hrp;

What convention do you follow in such cases?

Let's use _hrp and _witnesses for member variables, I will fix other warnings not from Nervos code

hewigovens avatar Aug 19 '22 02:08 hewigovens

@hewigovens we've changed those member variable names.

Thanks @hewigovens for adding more tests :)