feat: account impersonating improvements and eth_sendTransaction #137

AntonD3 · 2023-09-27T10:50:54Z

What 💻

Using playground bootloader with eth_call flag for impersonating
The same logic implemented for the eth_estimateGas
eth_sendTransaction method implemented

Why ✋

It allows to avoid any account checks, and use any address for the impersonating
It will return the correct amount of gas, when impersonating is used
For now, it works only with impersonated accounts, it allows to reuse existent js tools, for example, hardhat signer

Evidence 📷

MexicanAce · 2023-09-27T13:11:51Z

src/eth_test.rs

+///
+#[rpc]
+pub trait EthTestNodeNamespaceT {
+    #[rpc(name = "eth_sendTransaction")]


Now that this is implemented, can you enable the following e2e test? xdescribe -> describe

describe("hardhat_impersonateAccount & hardhat_stopImpersonatingAccount" ...

Where I can find this test?

At the bottom of the following file e2e-tests/test/hardhat-api.tests.ts

You can also search for the describe(...) I posted above in the repo and it'll show up

I see it, looks like my branch is outdated

MexicanAce · 2023-09-27T13:17:14Z

src/node.rs

+    /// # Returns
+    ///
+    /// A future that resolves to the hash of the transaction if successful, or an error if the transaction is invalid or execution fails.
+    fn send_transaction(


Please add unit tests for send_transaction

MexicanAce · 2023-09-27T13:20:56Z

Just as a general heads up, there's an open PR updating the meaning of current_miniblock, current_batch, current_timestamp #135

I'm guessing they'll be done first, but this will have slight conflicts with your changes (not much)

mm-zk · 2023-09-28T13:07:22Z

src/node.rs

+        };
+
+        l2_tx.set_input(bytes, hash);
+        if hash != l2_tx.hash() {


can you add a comment on when does this happen?

I think never, this code was stolen from the send_raw_transaction method. Maybe I will remove it

mm-zk · 2023-09-28T13:08:40Z

src/node.rs

+                if !reader
+                    .impersonated_accounts
+                    .contains(&l2_tx.common_data.initiator_address)
+                {


can we also print a clear error message, that this method is ONLY for impersonated accounts.
(and mention the initiator_address, and how many entries we have in impersonated_accounts list).

(as I'm worried that people will try using this method without knowing this)

mm-zk · 2023-09-28T13:10:18Z

src/node.rs

+            );
+            // Using the eth call here to avoid all the account checks
+            // TODO: think about fictive blocks in case of impersonating via eth call
+            TxExecutionMode::EthCall


the alternative, is to use the other DefaultAccount implementation in such case.

(as EthCall is in theory not supposed to be used for blocks that we commit)

Yes, but then the bootloader will do a check that the sender is an account and it's not in the kernel space

ah right, you actually want to have even more permissions...

The cool thing, is that we have a local copy of bootloader, so we can add some additional modes to do exactly that - and we can be explicit about it (instead of depending on a lucky side effect of EthCall..)

Agreed, I was planning to do that with preprocessing, but for now that's just a quick solution

mm-zk · 2023-09-28T13:15:00Z

src/node.rs

+
+            // If it was not eth call, we increase the batch by 1, but miniblock (and timestamp) by 2.
+            // You can look at insert_fictive_l2_block function in VM to see how this fake block is inserted.
+            if let TxExecutionMode::VerifyExecute = execution_mode {


uh, let's avoid this... I'm worried that we'll get bootloader in a bad spot.

What exactly? Do you mean a missing fictive block?

mm-zk · 2023-09-28T13:15:56Z

src/node.rs

+        };
+        // v = 27 corresponds to 0
+        let bytes = tx_req.get_signed_bytes(
+            &PackedEthSignature::from_rsv(&H256::default(), &H256::default(), 0),


could we use PackedEthSignature::default?

AntonD3 · 2023-10-19T08:59:43Z

@MexicanAce these changes were implemented in the boojum-integration branch in a better way. Do you think we should duplicate it to the main right now, or it's fine if we will merge it later from the boojum branch?

MexicanAce · 2023-10-19T09:24:51Z

@AntonD3 I think we're okay to wait for the boojum-integration implementation. Please still implement the feedback in this PR (especially unit tests)

* Initial new implementation * Fix compilation error * Implementation of add operation for big integers (matter-labs#136) * Implement Big UInt Left Shift (matter-labs#139) * Add `bigUIntShl` implementation * Add constants * Fix compilation * Implement Big UInt Right Shift (matter-labs#137) * Add `bigUIntShr` implementation * Add constants * Implement Big UInt Bitwise Or for modexp (matter-labs#135) * Implement bigUIntBitOr * Fix bigUIntBitOr * Fix bigUIntBitOr Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Fix missing closing brackets --------- Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Implement big uint conditional select for modexp (matter-labs#148) * Implement bigUIntCondSelect * Fix missing curly braces Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Implement Big UInt Right Shift (matter-labs#137) * Add `bigUIntShr` implementation * Add constants * Implement Big UInt Bitwise Or for modexp (matter-labs#135) * Implement bigUIntBitOr * Fix bigUIntBitOr * Fix bigUIntBitOr Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Fix missing closing brackets --------- Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> --------- Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Implement mul operation for big UInts (matter-labs#151) * First implementation of mul operation for bigints * Fix multiplication for big integers * Fix some merge issues * Improve comments and function docs * Delete whitespaces * Substraction with borrow (matter-labs#149) * First substraction draft * Fix compile problems * Working implementation * Updated code * Updated code * Update subtract implementation * Remove console_log * Add docs for function * Update function docs * Remove tests from from ModExp.yul * Fix typo * Restore horrible whitespaces to avoid an ugly merge conflict * Update precompiles/Modexp.yul Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Revert "Update precompiles/Modexp.yul" This reverts commit 582bc41a0bb7fb02a1a68717fdf83c6fe432d422. --------- Co-authored-by: Joaquín P. Centeno <jpcenteno@users.noreply.github.com> Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Refactor `modexp` reimplementation (matter-labs#156) * Make Big UInt API functions naming consistent * Refactor `bigUIntAdd` variable names * Refactor `bigUIntMul` variable names * Refactor `subLimbsWithBorrow` * Refactor `bigUintSubtractionWithBorrow` * Refactor `bigUIntAdd` * Fix `bigUIntSubWithBorrow` * Format `storeLimbValueAtOffset` * Refactor `bigUIntBitOr` Made it consistent with the rest of the code convention and naming * Refactor `bigUIntCondSelect` Made it consistent with the rest of the code convention and naming * Reorder `overflowingSubWithBorrow` * Move comment to modexp API Docs section * Biguint division (matter-labs#159) * Division draft * Non working draft * Fix compile errors * Use proper pointers for quotient and remainder * Add fix note, some more changes * Add comment * Implement `big_uint_bit_size` * Increase pointer to prevent it from steping over console_log * WIP divrem * Fix loop and zero initializer * Push test cases * Add other test case * Add docs and tests for `big_uint_inplace_or_1` * Fix bug related to bit shifting * Fix borrow return in big uint sub function * Delete playground file used for debugging * Fix sub with borrow function * Add playground again to check more big integer division tests * Remove playground used for testing * Write documentation for new shift functions * Improve naming and documentation for new helper functions * Rename bigUIntOrWith1 to bigUintInPlaceOrWith1 * Add tmp buffer parameters to bigUIntDivRem. Improve docs. * Simplify subLimbsWithBorrow Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Remove `mul` call from `bigUIntInPlaceOrWith1` Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Remove multiplications from copyBigUint Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Optimize bigUIntBitSize loop Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Simplify zeroWithLimbSizeAt --------- Co-authored-by: Francisco Krause Arnim <fkrausear@gmail.com> Co-authored-by: IAvecilla <iavecilla@fi.uba.ar> Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Implement mul mod operation for big UInts (matter-labs#161) * Division draft * Non working draft * Fix compile errors * Use proper pointers for quotient and remainder * Add fix note, some more changes * Add comment * Implement `big_uint_bit_size` * Increase pointer to prevent it from steping over console_log * WIP divrem * Fix loop and zero initializer * Push test cases * Add other test case * Add docs and tests for `big_uint_inplace_or_1` * Fix bug related to bit shifting * Fix borrow return in big uint sub function * Delete playground file used for debugging * Fix sub with borrow function * Add playground again to check more big integer division tests * Remove playground used for testing * Write documentation for new shift functions * Improve naming and documentation for new helper functions * Rename bigUIntOrWith1 to bigUintInPlaceOrWith1 * Add tmp buffer parameters to bigUIntDivRem. Improve docs. * Add big uint mul mod skeleton * Remove wrong comment * Update algorithm comment * Add limb size doubling and divide by two for mul mod operation * Functions to duplicate and halve limb size work in place * Use camelCase * Remove console_log * Add docs * Update doc --------- Co-authored-by: Francisco Krause Arnim <fkrausear@gmail.com> Co-authored-by: Joaquín P. Centeno <jpcenteno@users.noreply.github.com> * Add parseCallData function * Add function to left-pad big uints * Remove console log function * Change left padding functions for big uints to not work in place * Add `parseCalldata` function (matter-labs#168) * Remove redundant parse call data declaration * Free memory pointer (matter-labs#169) * Add free memory pointer function * Update precompiles/Modexp.yul Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Update precompiles/Modexp.yul Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> --------- Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> * Start parsing the input calldata * Correctly parse call data * Add left pad steps for modexp inputs * Add pad if needed function * Modexp for big UInts skeleton (matter-labs#164) * WIP: modexp skeleton * Use of mul mod function for big integers * imlement aux function to check if big uint is larger than 1 * minor fix * Restore modexp from target branch * Fix mul mod * Finish modexp implementation * Remove playground used for debugging * Update modexp with final state of modular exponentiation function * Fix merge issue * Change all names to camel case * fix typo Co-authored-by: Francisco Krause Arnim <56402156+fkrause98@users.noreply.github.com> --------- Co-authored-by: IAvecilla <iavecilla@fi.uba.ar> Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com> Co-authored-by: Francisco Krause Arnim <56402156+fkrause98@users.noreply.github.com> * Add simple integration * Fix calldata buffer in zero check * Uncomment checks for base cases * Fix result length to match with mod length * Fix condition in parse call data * Update test assertions with new test node updates * Add comment for tests with a temp patch * Fix modexp result length * Fix limb amount for modexp operands * Clean sratch buffers in each iteration * Clean sratch buffers for every operation * Remove unused functions * Delete free memory pointer usage and calculate pointers manually * Replace all mul operations for shifts to improve gas usage * Include basic optimizations * Add optimizations for reminder calculations * Add small improvement for main loop in modular exp * Add temporary fix for modexp test * Add modex reference script * Remove unnecesary memory stores * Reduce iterations in rem function * Compilation fix * Print gas used on tests * Add build script to create gas reports * Save gas used for each test of the precompiles * Add aux functions to write lines in each report * Merge main * Fix tests lint * Fix lint in test utils * Change L1 url --------- Co-authored-by: Nacho Avecilla <nachoavecilla@gmail.com> Co-authored-by: Joaquín Centeno <jpcenteno@users.noreply.github.com> Co-authored-by: Francisco Krause Arnim <56402156+fkrause98@users.noreply.github.com> Co-authored-by: Francisco Krause Arnim <fkrausear@gmail.com> Co-authored-by: IAvecilla <iavecilla@fi.uba.ar> Co-authored-by: Javier Chatruc <jrchatruc@gmail.com>

AntonD3 added 2 commits September 26, 2023 12:22

Account impersonation using eth call bootloader

3a93e58

eth_sendTransaction and sync estimateGas with impersonation

802b429

AntonD3 changed the title ~~Account impersonating improvements and eth_sendTransaction~~ feat: account impersonating improvements and eth_sendTransaction Sep 27, 2023

cargo fmt

8fce068

AntonD3 marked this pull request as ready for review September 27, 2023 13:00

AntonD3 requested a review from a team as a code owner September 27, 2023 13:00

MexicanAce reviewed Sep 27, 2023

View reviewed changes

Merge branch 'main' into ad-contract-impersonation

a1c6162

mm-zk reviewed Sep 28, 2023

View reviewed changes

MexicanAce closed this Oct 19, 2023

dutterbutter deleted the ad-contract-impersonation branch February 25, 2024 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: account impersonating improvements and eth_sendTransaction #137

feat: account impersonating improvements and eth_sendTransaction #137

AntonD3 commented Sep 27, 2023 •

edited

Loading

MexicanAce Sep 27, 2023

AntonD3 Sep 27, 2023

MexicanAce Sep 27, 2023

AntonD3 Sep 27, 2023

MexicanAce Sep 27, 2023

MexicanAce commented Sep 27, 2023

mm-zk Sep 28, 2023

AntonD3 Sep 28, 2023

mm-zk Sep 28, 2023

mm-zk Sep 28, 2023

AntonD3 Sep 28, 2023

mm-zk Sep 28, 2023

AntonD3 Sep 28, 2023

mm-zk Sep 28, 2023

AntonD3 Sep 28, 2023

mm-zk Sep 28, 2023

AntonD3 commented Oct 19, 2023

MexicanAce commented Oct 19, 2023

feat: account impersonating improvements and eth_sendTransaction #137

feat: account impersonating improvements and eth_sendTransaction #137

Conversation

AntonD3 commented Sep 27, 2023 • edited Loading

What 💻

Why ✋

Evidence 📷

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MexicanAce commented Sep 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AntonD3 commented Oct 19, 2023

MexicanAce commented Oct 19, 2023

AntonD3 commented Sep 27, 2023 •

edited

Loading