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: Discuss with core team the signature of eth_send_transaction #276

Closed
Tracked by #159
Eikix opened this issue Sep 5, 2023 · 6 comments
Closed
Tracked by #159
Labels
enhancement New feature or request smart-contract-logic A specific type of opcode that requires smart contract logic as opposed to pure cairo

Comments

@Eikix
Copy link
Member

Eikix commented Sep 5, 2023

Currently, in Kakarot Cairo0, for eth_send_transaction (entrypoint called during eth_sendRawTransaction via EOA) we use RLP::Decode and RLP::Encode functions. Why?

Current behaviour: https://github.com/kkrt-labs/kakarot/blob/c04cb72f040e059dfa3a8dc4ca24c6a68083409b/src/utils/eth_transaction.cairo#L28

Current Flow

sequenceDiagram
    participant User
    participant EOA
    User->>EOA: Sends raw tx payload (RLP encoded)
    EOA->>EOA: RLP::Decode to extract signature (r,s,v)
    EOA->>EOA: RLP::Encode payload without r,s,v to get signed payload
    EOA->>EOA: Hash resulting RLP encoded bytes to get tx hash
    EOA->>EOA: Perform eth_verify_signature with r, s, v, tx hash
    Note right of EOA: Costly in gas, but safe.
Loading

Then EOA is able to perform eth_verify_signature using r, s, v and the tx hash derived from the raw tx payload. This is very costly in gas, but safe.

Alternative number 1

sequenceDiagram
    participant User
    participant RustRPC
    participant EOA
    User->>RustRPC: Sends raw tx payload (RLP encoded)
    RustRPC->>EOA: Sends decoded payload as struct
    EOA->>EOA: Encode all args except r, s, v
    EOA->>EOA: Keccak the result of the encoding to get tx hash
    EOA->>EOA: Run eth_verify_signature with r, s, v, and tx hash
    EOA->>EOA: Proceed to Kakarot.eth_send_transaction if verification is successful
    Note right of EOA: Off-chain RLP decode, secure.
Loading

By using this workaround, we can perform off-chain the RLP decode without hindering security.

Alternative number 2: (not recommended ❌)

sequenceDiagram
    participant User
    participant RustRPC
    participant EOA
    User->>RustRPC: Sends raw tx payload (RLP encoded)
    RustRPC->>RustRPC: Decode raw tx, extract r,s,v, RLP-encode payload without r,s,v, and hash the result to get tx hash
    RustRPC->>EOA: Sends decoded payload as struct, including the tx hash
    EOA->>EOA: Run eth_verify_signature with r, s, v, tx hash
    EOA->>EOA: Proceed to Kakarot.eth_send_transaction
    Note right of EOA: Most efficient in gas.
Loading

Alternative number 3: (unclear about gas) (Recommended ✅)

sequenceDiagram
    participant User
    participant RustRPC
    participant EOA
    User->>RustRPC: Sends raw tx payload (RLP encoded)
    RustRPC->>RustRPC: Decode raw tx, extract r,s,v, then RLP-encode tx payload without r,s,v
    RustRPC->>EOA: Sends RLP-encoded payload (encoded without r,s,v fields), and the r, s, v fields.
    EOA->>EOA: Hash the RLP-encoded payload
    EOA->>EOA: Run eth_verify_signature with r, s, v, tx hash
    EOA->>EOA: Decode RLP-encoded tx payload
    EOA->>EOA: Proceed to Kakarot.eth_send_transaction
Loading

Recommendation:
I recommend number 3 alternative

@github-project-automation github-project-automation bot moved this to 🆕 Backlog in Kakarot on Starknet Sep 5, 2023
@Eikix Eikix changed the title Discuss with core team the signature of eth_send_transaction refactor: Discuss with core team the signature of eth_send_transaction Sep 5, 2023
@Eikix Eikix added enhancement New feature or request smart-contract-logic A specific type of opcode that requires smart contract logic as opposed to pure cairo labels Sep 5, 2023
@danilowhk
Copy link
Contributor

I am in favor of number 1, For number 2 a malicious user could directly send a transaction to Kakarot and have a decoded payload struct different from the tx hash info. (Maybe users cannot interact directly with Kakarot Contract if not the RPC, but I think its to big of a risk)

@ClementWalter
Copy link
Member

number 2 in this form is really naive. A serious number 2 would make the RPC signs their transaction, and fraud proof would squash a stake (not to say I'm in favor, but to say that 2 like that is ofc out of question)

@Eikix
Copy link
Member Author

Eikix commented Sep 5, 2023

number 2 in this form is really naive. A serious number 2 would make the RPC signs their transaction, and fraud proof would squash a stake (not to say I'm in favor, but to say that 2 like that is ofc out of question)

Interesting idea if we want one day to do a L3 or L4 with very low fees and fast TPS but accountable sequencer

@danilowhk
Copy link
Contributor

danilowhk commented Sep 5, 2023

I feel option 3 might be more efficient than option 1.

As encoding from the struct, would mean converting felts -> byte arrays, which need bit shift or division.

https://github.com/kkrt-labs/kakarot/blob/9a09abbb927af6ddba874e50f3ae91f1c581d309/src/utils/rlp.cairo#L154C13-L154C13

and for decoding to a struct, is converting bytes array -> felts, which is mostly multiplication and addition.

https://github.com/kkrt-labs/kakarot/blob/9a09abbb927af6ddba874e50f3ae91f1c581d309/src/utils/rlp.cairo#L62C1-L63C1

For cairo0 code, we are reusing the bytes_arrays generated to encode before verifying transactions, but if we have to encode the struct, we would need more operations similar to "encode_felt" and then later append them together with a function similar to "encode_list"

https://github.com/kkrt-labs/kakarot/blob/9a09abbb927af6ddba874e50f3ae91f1c581d309/src/utils/rlp.cairo#L95

@Eikix
Copy link
Member Author

Eikix commented Sep 8, 2023

Important update: we are gathering some bugs in the Kakarot v0 codebase, we need to make sure each issue and each PR in Kakarot-ssj is aware of the lists of known bugs. Look at this tracking issue everytime you take an issue and check your issue isn't targeted by a known bug. Will add this reminder in many places to make sure we keep track of known bugs.

@enitrat
Copy link
Contributor

enitrat commented Sep 22, 2023

LGTM @danilowhk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request smart-contract-logic A specific type of opcode that requires smart contract logic as opposed to pure cairo
Projects
No open projects
Archived in project
Development

No branches or pull requests

4 participants