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

refactor(gateway): user transaction improvements - hex serialization & query-only removal #436

Merged
merged 6 commits into from
Jan 15, 2025

Conversation

jbcaron
Copy link
Member

@jbcaron jbcaron commented Dec 20, 2024

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

  • Bugfix

What is the current behavior?

Resolves: #NA

What is the new behavior?

  • fix(gateway): serialize tip as hex in UserTransaction
  • refactor(gateway): remove query-only support from user transactions

Does this introduce a breaking change?

Yes: gateway API

Other information

@jbcaron jbcaron added the bug Report an issue or unexpected behavior label Dec 20, 2024
@jbcaron jbcaron self-assigned this Dec 20, 2024
@jbcaron jbcaron changed the title fix(gateway): serialize tip as hex in UserTransaction refactor(gateway): user transaction improvements - hex serialization & query-only removal Dec 20, 2024
@jbcaron jbcaron force-pushed the fix/fgw-user-transaction branch from 3dd1a16 to 1e78f22 Compare January 9, 2025 09:59
@jbcaron jbcaron force-pushed the fix/fgw-user-transaction branch from 1e78f22 to 539063c Compare January 9, 2025 16:58
let string_reader = std::io::Cursor::new(compressed_sierra_class.sierra_program);
let base64_decoder =
base64::read::DecoderReader::new(string_reader, &base64::engine::general_purpose::STANDARD);
let gzip_decoder = flate2::read::GzDecoder::new(base64_decoder);
Copy link
Member

Choose a reason for hiding this comment

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

aaaaaah
you should look at
#459

also I had no idea compressed sierra was a thing? i thought only legacy classes were compressed
that's new right?

CHANGELOG.md Outdated Show resolved Hide resolved
crates/madara/client/gateway/server/src/handler.rs Outdated Show resolved Hide resolved
crates/madara/primitives/class/src/lib.rs Show resolved Hide resolved
crates/madara/primitives/class/src/lib.rs Outdated Show resolved Hide resolved
crates/madara/primitives/gateway/src/user_transaction.rs Outdated Show resolved Hide resolved
crates/madara/primitives/gateway/src/user_transaction.rs Outdated Show resolved Hide resolved
@jbcaron jbcaron requested a review from Trantorian1 January 10, 2025 11:20
Comment on lines +61 to +68
/// Marker trait to restrict which types can be used as transaction results.
/// Only valid Starknet transaction responses implement this trait.
pub trait ValidTransactionResult {}

// Implement the marker trait for the three valid transaction result types
impl<T> ValidTransactionResult for AddInvokeTransactionResult<T> {}
impl<T> ValidTransactionResult for AddDeclareTransactionResult<T> {}
impl<T> ValidTransactionResult for AddDeployAccountTransactionResult<T> {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Consider sealing this trait by either making it private if does not need to be accessed outside of this crate, or a sub-trait of a private trait if it does.

Suggested change
/// Marker trait to restrict which types can be used as transaction results.
/// Only valid Starknet transaction responses implement this trait.
pub trait ValidTransactionResult {}
// Implement the marker trait for the three valid transaction result types
impl<T> ValidTransactionResult for AddInvokeTransactionResult<T> {}
impl<T> ValidTransactionResult for AddDeclareTransactionResult<T> {}
impl<T> ValidTransactionResult for AddDeployAccountTransactionResult<T> {}
/// Ensures [ValidTransactionResult] is accessible from outside the crate but cannot be implemented on any new structs
trait Sealed {}
/// Marker trait to restrict which types can be used as transaction results.
/// Only valid Starknet transaction responses implement this trait.
pub trait ValidTransactionResult: Sealed {}
// Implement the marker trait for the three valid transaction result types
impl<T> Sealed for AddInvokeTransactionResult<T> {}
impl<T> Sealed for AddDeclareTransactionResult<T> {}
impl<T> Sealed for AddDeployAccountTransactionResult<T> {}
impl<T> ValidTransactionResult for AddInvokeTransactionResult<T> {}
impl<T> ValidTransactionResult for AddDeclareTransactionResult<T> {}
impl<T> ValidTransactionResult for AddDeployAccountTransactionResult<T> {}

@antiyro antiyro merged commit dfafa02 into main Jan 15, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Report an issue or unexpected behavior
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants