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

EVM: Refactor FFI from rust strings to passing byte arrays #2733

Merged
merged 22 commits into from
Jan 16, 2024

Conversation

sieniven
Copy link
Member

@sieniven sieniven commented Nov 30, 2023

Summary

This PR refactors all exposed rust FFIs to pass byte vectors instead of rust strings, specifically when passing BTC internal hash storage representation into our Rust interface.

  • Fixes unsafe cpp std::string conversions to rust string, which will panic the node if it is not UTF-8 valid.
  • Cleans up and passes all byte vectors as BE representation with the exposed FFI.
  • Methods abstract conversions between BTC internal hash representations and BE byte arrays.
  • XVM struct is maintained to store miner address and EVM block hash as strings to maintain backwards compatibility. The hashes are only generated internally from BTC hash representations, and are guaranteed to be UTF-8 valid.
  • VM mapping stored on disk is also maintained to store hashes as string representation so that a re-index is not required. String conversions are safely generated by BTC hash representation.

Context

  1. Rust hashes are a fixed-sized uninterpreted hash types. By default we will pass byte vector in BE representation and stores it as big endian.
  2. Rust hashes as_fixed_bytes method returns the internal byte array containing the fixed hash.
  3. BTC hash representation internally stores bytes in little endian.

Implications

  • Storage

    • Database reindex required
    • Database reindex optional
    • Database reindex not required
    • None
  • Consensus

    • Network upgrade required
    • Includes backward compatible changes
    • Includes consensus workarounds
    • Includes consensus refactors
    • None

lib/ain-evm/src/core.rs Outdated Show resolved Hide resolved
@sieniven sieniven marked this pull request as ready for review December 1, 2023 09:45
Jouzo
Jouzo previously approved these changes Jan 5, 2024
Jouzo
Jouzo previously approved these changes Jan 12, 2024
@Jouzo Jouzo merged commit ce178e6 into master Jan 16, 2024
15 of 16 checks passed
@Jouzo Jouzo deleted the niven/ffi-cleanup branch January 16, 2024 08:26
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