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 type Transaction struct in rpc v7 and v8 #2458

Closed
hudem1 opened this issue Feb 11, 2025 · 1 comment
Closed

Refactor type Transaction struct in rpc v7 and v8 #2458

hudem1 opened this issue Feb 11, 2025 · 1 comment

Comments

@hudem1
Copy link
Contributor

hudem1 commented Feb 11, 2025

I was looking into refactoring the Transaction type for v7 and v8 that's used in several of their endpoints:

type Transaction struct {
	Hash                  *felt.Felt                   `json:"transaction_hash,omitempty"`
	Type                  TransactionType              `json:"type" validate:"required"`
	Version               *felt.Felt                   `json:"version,omitempty" validate:"required"`
	Nonce                 *felt.Felt                   `json:"nonce,omitempty" validate:"required_unless=Version 0x0"`
       ...
}

Pb

Transaction is the same for v7 and v8, but they have a slight difference with v6 (the required_if=Type INVOKE Version 0x3 for the SenderAddress field :

  • for v6:
    SenderAddress *felt.Felt json:"sender_address,omitempty" validate:"required_if=Type DECLARE,required_if=Type INVOKE Version 0x1"
  • for v7 and v8:
    SenderAddress *felt.Felt json:"sender_address,omitempty" validate:"required_if=Type DECLARE,required_if=Type INVOKE Version 0x1,required_if=Type INVOKE Version 0x3"

Idea

I thought about embedding v6's Tx type into v7 and v8's tx type and "override" SenderAddress field like this:

type Transaction struct {
   rpcv6.Transaction
   SenderAddress     *felt.Felt     `json:"sender_address,omitempty" validate:"required_if=Type DECLARE,required_if=Type INVOKE Version 0x1,required_if=Type INVOKE Version 0x3"`
}

By doing so, according to my tests on made-up types, it seems like the only sender_address key that will be kept in the returned json response is the one from v7/v8.

Questions

  1. I just have a worry concerning the json encoding validation due to the validate key. I am not sure when the validation happens; I looked at the code and it seems like it happens only during json unmarshaling (when receving requests) not when building responses. But that seems a bit strange to me (maybe I missed smth?), doesn't the validation also take place when building json when sending responses ?
  2. Then, in the code, when instantiating a Tx in v7 and v8, we have to be careful that the nested SenderAddress is not useful, only the overriding one is. Maybe that's not very intuitive, but that allows to reuse v6's code.
@hudem1 hudem1 mentioned this issue Feb 11, 2025
23 tasks
@rodrigo-pino
Copy link
Contributor

Talked with @hudem1, we decided not to go for it because it might hurt code legibility in the long run due the existence of two sender address fields in the v7 transaction struct.

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

No branches or pull requests

2 participants