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.

Sign up for GitHub

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

Jump to bottom

[Dev/UnderAudit]: Dev<->Master #3120

Merged
merged 24 commits into from
Oct 13, 2023
Merged

[Dev/UnderAudit]: Dev<->Master #3120

merged 24 commits into from
Oct 13, 2023

Conversation

Milerius
Copy link
Collaborator

@Milerius Milerius commented Apr 26, 2023 •
edited by satoshiotomakan
Loading

Description

This Pull Request introduces several sweeping changes to the Wallet Core, primarily centering on the shift from C++ to Rust for improved performance, security, and maintenance:

  1. Elliptic Curves in Rust: We've successfully migrated most of the currently supported elliptic curves to Rust. Implementation details are as follows:
  2. Wallet Core Redesign: Exploiting Rust's robust type system, the Wallet Core design has been massively enhanced to promote easier implementation of features and long-term code maintainability.
  3. Ethereum Transition: We've shifted the main Ethereum functionality from C++ to Rust. The implementation encompasses Ethereum's basic operations, RLP and ABI interfaces, as well as EIP712 message signing. As it stands now, the Ethereum transition is fully executed.
  4. Bitcoin Transition: Progress is underway to move Bitcoin functionality from C++ to Rust. So far, this involves Bitcoin's basic operations and interface redesign. Pending tasks include:
    • Establishing a bridge between the legacy and new Bitcoin protobuf interfaces for seamless transition.
    • Shifting all tests to Rust.
    • Implementing other Bitcoin-related functionality in Rust.

Progress

Breaking changes

  • VeChain and Theta no longer support JSON signing. Initially, the chains inherited Ethereum::Entry, but Entry::signJSON was not overridden with the chain specific implementation.
  • UserOperation signed transaction (JSON representation) now has the sender: Address field normalized with EIP55 checksum.
  • Removed Ethereum::RLP::decode functionality as it's not used in C++.
  • Ethereum and all EVM-compatible chains no longer support TWTransactionCompilerBuildInput.

Deprecated

  • TWEthereumAbiFunction and all methods.
  • TWEthereumAbiEncode. Consider using TWEthereumAbiEncodeFunction through Protobuf interface instead.
  • TWEthereumAbiDecodeOutput. Consider using TWEthereumAbiDecodeParams through Protobuf interface instead.
  • TWEthereumAbiDecodeCall. Consider using TWEthereumAbiDecodeContractCall through Protobuf interface instead.
  • TWEthereumAbiValueDecodeValue, TWEthereumAbiValueDecodeArray and all TWEthereumAbiValueDecode*. Consider using TWEthereumAbiDecodeValue through Protobuf interface instead.

TODO

  • Consider implement tw_ethereum::Entry::sign_json - we need to replace quick-protobuf with rust-protobuf as it's the only crate that supports Protobuf -> JSON conversion
  • It would be great to implement transaction decoding for every chain in Rust.
  • Enable secp256k1, nist256p1, ed25519, ed25519-blake2b, ed25519-cardano-extended elliptic curves in Rust.

@satoshiotomakan satoshiotomakan linked an issue Apr 26, 2023 that may be closed by this pull request
[Rust]: Implement Private and Public keys #3093
Closed
6 tasks
@Milerius Milerius requested review from a team, BSCSecChef and defisecb and removed request for a team April 26, 2023 11:14
@Milerius Milerius marked this pull request as draft April 26, 2023 12:40
@satoshiotomakan satoshiotomakan mentioned this pull request Jun 2, 2023
Merged
satoshiotomakan and others added 17 commits October 6, 2023 09:56
@satoshiotomakan
[Rust]: Implement Private and Public keys (#3107)
a565082
@satoshiotomakan
[Rust]: Add cargo fuzz tests (#3126)
b6f2085
@satoshiotomakan
[Rust]: Implement Ed25519 curve (#3146)
df7cd99
@satoshiotomakan
[Rust]: Implement Curve25519 (#3163)
67c9429 
* [wip] Add ed25519 tests

* [wip] Implement signing, getting public from private key

* TODO: Implement verifying signature
* TODO: Add `PrivateKeyExtended`, `PublicKeyExtended`

* [wip] Implement signature verifying

* [wip] Implement Cardano extended private and public keys

* [wip] Integrate Ed25519 curves into C++

* [wip] Minor changes

* Remove unused dependencies
* Rename `SigningHash` to `SigningMessage`

* Fix tw_keypair/fuzz

* Move cardano specific keypair to `modifications` module

* [wip] Add ed25519 KeyPair's

* [wip] Fix code coverage

* [wip] Minor changes

* Rename `Hash512` trait to `Hasher512`

* [wip] Document the new added structures

* [wip] Add test cases generated using `trezor-crypto`

* Fix generating Cardano extended public key
* Fix signing using Cardano extended private key

* [r2r] Optimize, add doc comments, improve tests

* [wip] Don't use sccache for WASM target

* [wip] Ignore sccache

* [wip] Verbose WASM tests

* [wip] Use opt-level=2

* [wip] Use opt-level=2 for all tests

* [wip] Enable sccache, run only WASM tests with opt-level=2

* Add `tools/rust-test` script

* [r2r] Enable all CI workflows and checks

* [r2r] Fix Clippy warnings

* [r2r] Avoid implementing `From<&'static str'>` for key pairs

* [r2r] Fix PR issues

* Add `SplitHash<L, R>` type alias
* Rename `tw_keypair::Error` into `KeyPairError`
* Add `KeyPairResult<T>` type alias

* [wip] Add Curve25519 impl

* TODO cover with tests
* Document
* Integrate into C++

* [wip] Add two tests

* [wip] Add Waves signing cases

* [wip] Add Private to Public test cases

* [wip] Add missing JSON

* [wip] Cover with more tests

* [wip] Integrate Curve25519 into CPP

* [wip] Add doc comments

* [wip] Push missing PrivateKey and PublicKey files

* [r2r] Minor changes
@satoshiotomakan
[Rust]: Implement nist256p1 curve (#3169)
3ef9f53
@satoshiotomakan
Resolve dev<-master conflicts (#3215)
ae387f7
@satoshiotomakan
Merge master into dev (#3279)
cef220b
@satoshiotomakan
Sync dev with master (#3308)
9b50c29
@satoshiotomakan @rsrbk
@hewigovens @lamafab @Milerius @MaximPestryakov @fsabuhi @Balashov152
Sync dev with master (#3313)
3d9d407 
* [r2r]: Add ThetaFuel EVM compatible chain

* [r2r]: Bump wc-kotlin version to 3.1.38 (#3218)

* [ERC-4337] Fix user operation hash mistake (#3228)

* [ERC-4337] Fix user operation hash mistake

* Update TestBarz.kt

---------

Co-authored-by: hewigovens <[email protected]>

* [r2r] Update starknet-ff crate (#3230)

* [Sync]: Apply changes from DeFi wallet-core (#3225)

* [Barz] Format signature API changes (#3241)

* [Barz] Public API refactoring (#3212)

* Getting rid of attestationObject in Barz public API

* remove duplicates

* Update Barz.cpp

* Update Barz.cpp

* Update BarzTests.swift

* update tests

* Update WebAuthnTests.cpp

* Update TestBarz.kt

* Update TestWebAuthn.kt

* update android tests

* Update TestBarz.kt

* Update TestBarz.kt

* Update TestBarz.kt

* fix nullability

* tests update

* Add support for Taproot and BRC20 (#3233)

* [Barz] Add TokenReceiverFacet support (#3240)

* [CFX]: Add support for Conflux eSpace (#3259)

* [License]: update to apache 2.0 (#3256)

* [Barz] Execute batch (#3257)

* [Kotlin] Fixed GH Action (#3263)

* feat(evm/thorchain): use depositWithExpiry for regular transfer (#3266)

* feat(swift): update to 3.2.1 (#3269)

* Add iOS and android tests to check if new methods available (#3267)

* [WASM] Fixed CoinType.deriveAddressFromPublicKeyAndDerivation (#3271)

* [Polkadot]: Refactor Polkadot to support any Substrate chain (#3261)

* [Acala]: Add Acala and AcalaEVM chains (#3274)

* [Kotlin] Added UInt value to enums (#3276)

Co-authored-by: Sztergbaum Roman <[email protected]>

* fix(bitcoin): Fix `TWAnySignerPlan` (#3278)

* [README]: Update README.md (#3280)

Removed broken IFWallet link
Updated supported blockchain count according to registry.md
Corrected some typos and grammatical mistakes

* [Chain/EVM]: add opbnb testnet (#3291)

* [Barz] Upgrade to the new Barz contract constructor (#3288)

* [Polkadot]: Fix `Staking::Bond` and `Staking::BondAndNominate` (#3293)

* [Webauthn] Add some helpers to extract P256 values (#3292)

* [Cardano] Support utxo with legacy(byron) address (#3284)

* [Cardano]: Fix signing in JS (#3299)

---------

Co-authored-by: Ruslan Serebriakov <[email protected]>
Co-authored-by: hewigovens <[email protected]>
Co-authored-by: Fabio Lama <[email protected]>
Co-authored-by: Sztergbaum Roman <[email protected]>
Co-authored-by: Maxim Pestryakov <[email protected]>
Co-authored-by: Sabuhi Fatalizada <[email protected]>
Co-authored-by: Sergey Balashov <[email protected]>
@satoshiotomakan
[Blockchain]: Move Ethereum implementation to Rust (#3322)
5f3a42f
@satoshiotomakan
[Rust]: Replace C++ RLP implementation (#3350)
0a3ba16
@satoshiotomakan @Milerius
@lamafab @rsrbk
Sync dev with master (#3358)
a104f8d 
* [NewChain/Base]: add base blockchain (#3342)

* [Aptos]: Add `transfer_coins` function call (#3344)

* feat(aptos): Add `TokenTransferCoinsMessage` to call `aptos_account::transfer_coins` function

* Add test with testnet transaction

* feat(aptos): Add mainnet test

* feat(aptos): Fix comment

* [Bitcoin] Fix `TWBitcoinFeeCalculateFee` (for JavaScript) (#3346)

* [Barz] Allow deriving multiple addresses from a single public key (#3340)

* [Barz] Allow deriving multiple addresses from a single public key

* fix tests

* Update BarzTests.swift

* update tests

* update tests

* update tests

* [ETH]: Handle `tuple[]` ABI type parameter (#3352)

---------

Co-authored-by: Sztergbaum Roman <[email protected]>
Co-authored-by: Fabio Lama <[email protected]>
Co-authored-by: Ruslan Serebriakov <[email protected]>
@lamafab @satoshiotomakan
[Rust/Bitcoin/Utxo] Great Migration towards tw_bitcoin and tw_utxo (
c37bd9d 
#3382)
@lamafab @satoshiotomakan
Sync dev with master (#3435)
148af54
@satoshiotomakan
[Rust/Eth]: Replace C++ Ethereum ABI implementation (#3440)
993a6de
@satoshiotomakan
[Rust/Eth]: Replace C++ Ethereum MessageSigner (#3458)
93c1c1d
@satoshiotomakan
[Rust/Eth]: Add fuzz tests (#3463)
6f7e78e
@Milerius Milerius marked this pull request as ready for review October 13, 2023 10:35
@Milerius Milerius merged commit b530432 into master Oct 13, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@satoshiotomakan satoshiotomakan satoshiotomakan approved these changes

@lamafab lamafab lamafab approved these changes

@BSCSecChef BSCSecChef Awaiting requested review from BSCSecChef

@defisecb defisecb Awaiting requested review from defisecb

Assignees
No one assigned
Labels
do not merge under audit
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Rust]: Implement Private and Public keys
3 participants
@Milerius @lamafab @satoshiotomakan