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

[Rust/Aptos]: Move Aptos blockchain into Rust #3497

Merged
merged 62 commits into from
Nov 15, 2023
Merged

[Rust/Aptos]: Move Aptos blockchain into Rust #3497

merged 62 commits into from
Nov 15, 2023

Conversation

Milerius
Copy link
Collaborator

@Milerius Milerius commented Oct 23, 2023

Description

Move aptos C++ to Rust

Breaking change:

  • Deprecate blind sign through encoded hex, accept only payload json now

How to test

Run swift,android,wasm,c++,rust unit tests

Types of changes

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.

If you're adding a new blockchain

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

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.

Great work! I've left a few comments what could be reworked in my opinion

rust/tw_aptos/Cargo.toml Show resolved Hide resolved
Comment on lines 10 to 12
pub trait AptosContext {
type Address: AptosAddress;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that EvmContext confuses. The reason why we used EvmContext with an EvmContext::Address associated type is that there are some EVM forks that have different address formats.
For example, Ronin supports ronin:ec49280228b0d05aa8e8b756503254e1ee7835ab, but other EVM chains do not.
As far as I know, Aptos has only one address format (at least for now), so AptosContext and AptosAddress are not needed.
Therefore, the signer can be just:

pub struct Signer;

impl Signer {
  pub fn sign_proto_impl(...) {
    // ...
    let sender = Address::from_str(&input.sender)?;
    // ...
 }
}

rust/tw_aptos/src/transaction.rs Outdated Show resolved Hide resolved
rust/tw_aptos/src/transaction_builder.rs Outdated Show resolved Hide resolved
rust/tw_aptos/src/transaction_builder.rs Outdated Show resolved Hide resolved
@trustwallet trustwallet deleted a comment from momka1234 Nov 6, 2023
@trustwallet trustwallet deleted a comment from momka1234 Nov 6, 2023
@Milerius Milerius changed the title [Rust/Aptos]: WIP [Rust/Aptos]: Move aptos blockchain to rust Nov 13, 2023
@Milerius Milerius marked this pull request as ready for review November 13, 2023 16:47
@Milerius Milerius requested a review from lamafab as a code owner November 13, 2023 16:47
@Milerius Milerius changed the title [Rust/Aptos]: Move aptos blockchain to rust [Rust/Aptos]: Move aptos blockchain into Rust Nov 13, 2023
@Milerius Milerius changed the title [Rust/Aptos]: Move aptos blockchain into Rust [Rust/Aptos]: Move Aptos blockchain into Rust Nov 13, 2023
@Milerius Milerius merged commit 4a5fc03 into master Nov 15, 2023
12 checks passed
@Milerius Milerius deleted the m/aptos_rust branch November 15, 2023 19:05
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.

2 participants