-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Pactus]: Support Pactus Blockchain #4057
Conversation
I hope this message finds you well! If you have a moment, I would greatly appreciate your review of this pull request. Please let us know if there’s anything we might have missed that needs attention before we proceed with the merge. Thank you very much for your assistance! |
Hey @b00f He will review the PR next week - thank you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @b00f, thank you for the chain addition! Overall the code is great, I only have minor changes and improvements.
Could you please also add mobile tests via
cd codegen
codegen/bin/newcoin-mobile-tests pactus
@satoshiotomakan Thank you for taking the time to review this PR. I believe all of your comments have been addressed. Could you please double-check and resolve those that are confirmed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Hi @b00f, could you please fix Lines 25 to 46 in 4b203e0
wallet-core/swift/Tests/Blockchains/EthereumTests.swift Lines 22 to 42 in 4b203e0
|
@b00f please also fix the following assertion o iOS:
|
@satoshiotomakan it's done. |
@satoshiotomakan |
...oid/app/src/androidTest/java/com/trustwallet/core/app/blockchains/pactus/TestPactusSigner.kt
Outdated
Show resolved
Hide resolved
...oid/app/src/androidTest/java/com/trustwallet/core/app/blockchains/pactus/TestPactusSigner.kt
Outdated
Show resolved
Hide resolved
All tests have been updated with data from broadcasted transactions on the mainnet. Transactions are available here:
@satoshiotomakan please check. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@satoshiotomakan There was a typo in the Android test, which has been fixed. Please re-run the tests. Apologies for the mistake. |
Hi @b00f, if you want the Pactus Blockchain to be integrated into TrustWallet app, please submit a request to |
@satoshiotomakan and the Wallet-Core project team, Thank you for this amazing project and for carefully reviewing this PR and helping us correct our mistakes. I will definitely submit the request, and we hope to see Pactus in the Trust Wallet application soon. |
Description
This PR adds support for the Pactus Blockchain in TrustWalletCore.
It follows the Integration Criteria highlighted by TrustWalletCore team.
How to test
This PR includes all the necessary tests to ensure the integration is done properly.
We have also compared the results with the native wallet in Pactus, which is available here and here.
This PR fixes #4056 .
Types of changes
New feature (non-breaking change which adds functionality)
Checklist
Integration Checklist
registry.json
.pushd codegen-v2 && cargo run -- new-blockchain <coinid> && popd
to generate blockchain skeleton and configure integration tests.tools/generate-files
.pushd rust && cargo check --tests && popd
to check if Rust codebase compiles successfully.make -Cbuild -j12 tests TrezorCryptoTests
to check if C++ codebase compiles successfully.Curve
,Hasher
etc., in C++ and Rust, as necessary.rust/chains/tw_<coinid>/src/entry.rs
.rust/chains/tw_<coinid>/src/address.rs
.rust/chains/tw_<coinid>/src/transaction.rs
.rust/chains/tw_<coinid>/src/signer.rs
.rust/chains/tw_<coinid>/src/compiler.rs
.rust/chains/tw_<coinid>/tests
directory.rust/tw_any_coin/tests/chains/<coinid>
tests/chains/<CoinId>
.CoinAddressDerivationTests.cpp
andcoin_address_derivation_test.rs
,CoinAddressValidationTests.cpp
,TWHRPTests.cpp
,CoinAddressDerivationTests.kt
,CoinAddressDerivationTests.swift
.