Skip to content
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

Merged
merged 40 commits into from
Nov 7, 2024

Conversation

b00f
Copy link
Contributor

@b00f b00f commented Oct 7, 2024

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

  • Create pull request as draft initially, unless its complete.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • If there is a related Issue, mention it in the description.
  • I have read the guidelines for adding a new blockchain.

Integration Checklist

  • Coin Definition:
    • Add the coin definition to registry.json.
    • Execute pushd codegen-v2 && cargo run -- new-blockchain <coinid> && popd to generate blockchain skeleton and configure integration tests.
    • Execute tools/generate-files.
    • Execute pushd rust && cargo check --tests && popd to check if Rust codebase compiles successfully.
    • Execute make -Cbuild -j12 tests TrezorCryptoTests to check if C++ codebase compiles successfully.
  • Add relevant constants in Curve, Hasher etc., in C++ and Rust, as necessary.
  • Implement functionality in Rust.
    • Entry at rust/chains/tw_<coinid>/src/entry.rs.
    • Address at rust/chains/tw_<coinid>/src/address.rs.
    • Transaction (if necessary) at rust/chains/tw_<coinid>/src/transaction.rs.
    • Signer at rust/chains/tw_<coinid>/src/signer.rs.
    • Compiler at rust/chains/tw_<coinid>/src/compiler.rs.
    • Write unit tests. Put them in the rust/chains/tw_<coinid>/tests directory.
    • Write Rust integration tests. Put them in a subfolder of rust/tw_any_coin/tests/chains/<coinid>
      • Transaction signing tests, at least one mainnet transaction test.
      • Add stake, unstake, get rewards tests if the blockchain is PoS like.
  • Write C++ integration tests. Put them in a subfolder of tests/chains/<CoinId>.
  • Validate generated code in Android an iOS projects. Write integration tests for each.
  • Extend central derivation and validation tests: make sure the following tests are extended with the new coin: CoinAddressDerivationTests.cpp and
    coin_address_derivation_test.rs,
    CoinAddressValidationTests.cpp,
    TWHRPTests.cpp,
    CoinAddressDerivationTests.kt,
    CoinAddressDerivationTests.swift.
  • Upload coin icon to trustwallet/assets if necessary.

@b00f b00f marked this pull request as draft October 7, 2024 07:01
@b00f b00f changed the title Pactus Support Pactus Blockchain Oct 7, 2024
@b00f b00f changed the title Support Pactus Blockchain [Pactus]: Support Pactus Blockchain Oct 7, 2024
@b00f b00f marked this pull request as ready for review October 9, 2024 12:50
@b00f
Copy link
Contributor Author

b00f commented Oct 9, 2024

@philiparthurmoore

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!

@Milerius
Copy link
Collaborator

Hey @b00f He will review the PR next week - thank you

Copy link
Collaborator

@satoshiotomakan satoshiotomakan left a 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

rust/chains/tw_pactus/src/address.rs Outdated Show resolved Hide resolved
rust/chains/tw_pactus/src/encoder/var_int.rs Outdated Show resolved Hide resolved
rust/chains/tw_pactus/src/transaction.rs Outdated Show resolved Hide resolved
rust/chains/tw_pactus/src/transaction.rs Outdated Show resolved Hide resolved
rust/chains/tw_pactus/src/transaction.rs Outdated Show resolved Hide resolved
rust/chains/tw_pactus/src/transaction.rs Outdated Show resolved Hide resolved
rust/chains/tw_pactus/src/transaction.rs Outdated Show resolved Hide resolved
rust/tw_tests/tests/chains/pactus/pactus_sign.rs Outdated Show resolved Hide resolved
tests/chains/Pactus/SignerTests.cpp Outdated Show resolved Hide resolved
tests/chains/Pactus/TestCases.h Outdated Show resolved Hide resolved
@b00f
Copy link
Contributor Author

b00f commented Oct 16, 2024

@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?

Copy link
Collaborator

@satoshiotomakan satoshiotomakan left a comment

Choose a reason for hiding this comment

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

LGTM!

@satoshiotomakan
Copy link
Collaborator

Hi @b00f, could you please fix cargo fmt warnings, fix cargo clippy in advance, make sure all Rust tests pass.
Please also take a look at these Android, Swift test examples, and change your tests accordingly:

@Test
fun testEthereumTransactionSigning() {
val signingInput = Ethereum.SigningInput.newBuilder()
signingInput.apply {
privateKey = ByteString.copyFrom(PrivateKey("0x4646464646464646464646464646464646464646464646464646464646464646".toHexByteArray()).data())
toAddress = "0x3535353535353535353535353535353535353535"
chainId = ByteString.copyFrom("0x1".toHexByteArray())
nonce = ByteString.copyFrom("0x9".toHexByteArray())
// txMode not set, Legacy is the default
gasPrice = ByteString.copyFrom("0x04a817c800".toHexByteArray())
gasLimit = ByteString.copyFrom("0x5208".toHexByteArray())
transaction = Ethereum.Transaction.newBuilder().apply {
transfer = Ethereum.Transaction.Transfer.newBuilder().apply {
amount = ByteString.copyFrom("0x0de0b6b3a7640000".toHexByteArray())
}.build()
}.build()
}
val output = AnySigner.sign(signingInput.build(), ETHEREUM, SigningOutput.parser())
assertEquals(Numeric.toHexString(output.encoded.toByteArray()), "0xf86c098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a76400008025a028ef61340bd939bc2195fe537567866003e1a15d3c71ff63e1590620aa636276a067cbe9d8997f761aecb703304b3800ccf555c9f3dc64214b297fb1966a3b6d83")
}

func testSigner() throws {
let input = EthereumSigningInput.with {
$0.chainID = Data(hexString: "01")!
$0.nonce = Data(hexString: "09")!
// txMode not set, Legacy is the default
$0.gasPrice = Data(hexString: "04a817c800")!
$0.gasLimit = Data(hexString: "5208")!
$0.toAddress = "0x3535353535353535353535353535353535353535"
$0.privateKey = Data(hexString: "0x4646464646464646464646464646464646464646464646464646464646464646")!
$0.transaction = EthereumTransaction.with {
$0.transfer = EthereumTransaction.Transfer.with {
$0.amount = Data(hexString: "0de0b6b3a7640000")!
}
}
}
let output: EthereumSigningOutput = AnySigner.sign(input: input, coin: .ethereum)
XCTAssertEqual(try input.serializedData().hexString, "0a0101120109220504a817c8002a025208422a3078333533353335333533353335333533353335333533353335333533353335333533353335333533354a204646464646464646464646464646464646464646464646464646464646464646520c0a0a0a080de0b6b3a7640000")
XCTAssertEqual(output.encoded.hexString, "f86c098504a817c800825208943535353535353535353535353535353535353535880de0b6b3a76400008025a028ef61340bd939bc2195fe537567866003e1a15d3c71ff63e1590620aa636276a067cbe9d8997f761aecb703304b3800ccf555c9f3dc64214b297fb1966a3b6d83")
}

@satoshiotomakan
Copy link
Collaborator

@b00f please also fix the following assertion o iOS:

testAddress, XCTAssertEqual failed: ("95794161374b22c696dabb98e93f6ca9300b22f3b904921fbf560bb72145f4fa") is not equal to ("0x95794161374b22c696dabb98e93f6ca9300b22f3b904921fbf560bb72145f4fa")

@b00f
Copy link
Contributor Author

b00f commented Oct 22, 2024

@satoshiotomakan it's done.

@b00f
Copy link
Contributor Author

b00f commented Oct 29, 2024

@satoshiotomakan
The tests in Rust are now compatible with mainnet transactions. Additionally, we have implemented the Bond Payload, which is also covered by tests.

@b00f
Copy link
Contributor Author

b00f commented Nov 1, 2024

All tests have been updated with data from broadcasted transactions on the mainnet. Transactions are available here:

@satoshiotomakan please check. Thanks

satoshiotomakan
satoshiotomakan previously approved these changes Nov 5, 2024
Copy link
Collaborator

@satoshiotomakan satoshiotomakan left a comment

Choose a reason for hiding this comment

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

LGTM

@b00f
Copy link
Contributor Author

b00f commented Nov 5, 2024

@satoshiotomakan There was a typo in the Android test, which has been fixed. Please re-run the tests. Apologies for the mistake.

@satoshiotomakan satoshiotomakan merged commit 6be89c9 into trustwallet:master Nov 7, 2024
12 checks passed
@satoshiotomakan
Copy link
Collaborator

Hi @b00f, if you want the Pactus Blockchain to be integrated into TrustWallet app, please submit a request to
https://support.trustwallet.com/en/support/tickets/new

@b00f b00f deleted the pactus branch November 8, 2024 14:13
@b00f
Copy link
Contributor Author

b00f commented Nov 8, 2024

@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.

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.

Add support for Pactus Blockchain
4 participants