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

Split transaction builders #2

Merged
merged 31 commits into from
Aug 28, 2024
Merged

Conversation

EdsonAlcala
Copy link
Contributor

@EdsonAlcala EdsonAlcala commented Aug 22, 2024

This PR adds several changes to the current code base, in my opinion there are some benefits that are described in more detail below, while it is also preserving the current API, with some minor changes.

Here is the description of the changes made:

  • Included rust config files:

    • .rustfmt.toml
    • .clippy.toml
    • rust-toolchain.toml
  • Included justfile

  • Included licenses

    • LICENSE-APACHE
    • LICENSE-MIT
  • Included Github workflows

    • A Github workflow was included to improve the quality of the code base. The Github workflow includes:
      • build
      • build wasm
      • integration tests
      • tests
  • Split transaction builders

    • The Near transaction building was abstracted into its own file(builder), called NearTransactionBuilder, this has the advantage of having the builder logic in a single place with the required fields for a Near transaction. This can be beneficial because with the current approach, users can fill fields that are not required for a Near Transaction and can be confusing.

    • The EVM Transaction building was abstracted into its own file too, called EVMTransactionBuilder. This share the same benefits of the NearTransactionBuilder, including users only filling what is required for an EVM transaction.

    • The NearTransactionBuilder now "builds" a NearTransaction type, this structure was included to provide additional capabilities like encoding with signature or simply encoding. The idea behind this is to allow users to not only encode transactions but also to enable the integration with the MPC signer and encode the signature within the smart contract. Facilitating the encoding and signature will allow users to simple request a signed transaction that can be easily propagated. The PR includes the signature encoding for the EVMTransaction, however a future PR will include the support for the NearTransaction. This change was required since the current implementation only returns a vec, by abstracting this into its own struct we can do this like:

let evm_transaction = EVMTransactionBuilder::new()
            .chain_id(chain_id)
            .nonce(nonce)
            .max_priority_fee_per_gas(MAX_PRIORITY_FEE_PER_GAS)
            .max_fee_per_gas(MAX_FEE_PER_GAS)
            .gas_limit(GAS_LIMIT)
            .to(to_address)
            .value(value)
            .input(input.to_vec())
            .access_list(vec![])
            .build();

// Encode the transaction with EIP-1559 prefix
let evm_tx_encoded = evm_transaction.build_for_signing();

// Hash the encoded transaction
let evm_tx_hash = keccak256(&evm_tx_encoded);

// Sign the transaction hash
let signature = signer.sign_hash(&evm_tx_hash).await?;

let signature: OmniSignature = OmniSignature {
        v: signature.v().to_u64(),
        r: signature.r().to_be_bytes::<32>().to_vec(),
        s: signature.s().to_be_bytes::<32>().to_vec(),
};

// Add the signature to allow to directly propagate the payload
let evm_tx_encoded_with_signature = evm_transaction.build_with_signature(&signature);
  • The current API is almost preserved by including a TransactionBuilder wrapper where users can simply specify the transaction type and have access directly to the specific methods of that transaction type. For instance for EVM:
Screenshot 2024-08-22 at 17 54 51

Similar with Near:

Screenshot 2024-08-22 at 17 53 26
  • The transaction builders now enforce calling the build function. Since this is the only way where the Transaction types are created (EVMTransaction or NearTransaction and more in the future). Facilitating their implementation and removing posible errors from implementers.

  • Included integration tests for EVMTransaction against Anvil.

  • Included unit tests of encoding against alloy.

  • Included unit tests of encoding against Near primitives.

  • Removed Near primitives dependency.

  • Removed Near crypto dependency.

Limitations

  • A limitation of this current PR is the lack of support for the priority fee that was included in Transaction priority fee near/NEPs#541. This will be included in a future PR.

  • This PR only supports EIP 1559 transaction types. For support of legacy transactions, please share your thoughts.

Future Work

[ ] Include integration tests for Near
[ ] Support building with signature for NearTransaction
[ ] Create contribution guidelines
[ ] Create code of conduct
[ ] Create changelog
[ ] Add release workflow to publish the library as a crate
[ ] Write documentation
[ ] Document the code base
[ ] Create a pull request template and issue template
[ ] Test the following Near actions:

  • CreateAccountAction
  • DeployContractAction
  • FunctionCallAction
  • StakeAction

[ ] Support more Near actions:

  • AddKeyAction
  • DeleteKeyAction
  • DeleteAccountAction
  • SignedDelegateAction

[ ] Support priority fee in Near transactions, review near/NEPs#541
[ ] Create a examples repository with a structure as:

Screenshot 2024-08-22 at 15 34 30

@ilblackdragon ilblackdragon merged commit d571478 into ilblackdragon:master Aug 28, 2024
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