feat: send legacy balance to L1

[arad-starkware]

This change is 

[arad-starkware]

• feat: send legacy balance to L1 #17 👈 (View in Graphite)
• feat: migrate function implementation #16 : 1 other dependent PR (#26 )
• test: use snforge::Token #25
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[noa-starkware]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @arad-starkware)

---

packages/usdc_migration/src/usdc_migration.cairo line 31 at r1 (raw file):

        l1_transfer_unit: u256,
        /// L1 USDC token address.
        l1_usdc_token_address: EthAddress,

all should be initialized in constructor

---

packages/usdc_migration/src/usdc_migration.cairo line 83 at r1 (raw file):

            self.emit(USDCMigrationEvents::USDCMigratedEvent { amount, user: caller_address });
            self._recycle(:contract_address);

Why we need that as param?

Code quote:

contract_address

---

packages/usdc_migration/src/usdc_migration.cairo line 86 at r1 (raw file):

        }

        fn recycle(ref self: ContractState) {

Didnt we decide its not needed?

---

packages/usdc_migration/src/usdc_migration.cairo line 116 at r1 (raw file):

                starkgate_dispatcher
                    .initiate_token_withdraw(
                        l1_token: l1_usdc_token_address, :l1_recipient, :amount,

where is the unit size?

[arad-starkware]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @arad-starkware and @noa-starkware)

---

packages/usdc_migration/src/usdc_migration.cairo line 31 at r1 (raw file):

Previously, noa-starkware wrote…

all should be initialized in constructor

don't you do that in set_threshold?
I want to keep this as a draft until that one is merged (or the PRs below this one are so I can move it above set_threshold)

---

packages/usdc_migration/src/usdc_migration.cairo line 83 at r1 (raw file):

Previously, noa-starkware wrote…

Why we need that as param?

to avoid calling it again

---

packages/usdc_migration/src/usdc_migration.cairo line 86 at r1 (raw file):

Previously, noa-starkware wrote…

Didnt we decide its not needed?

depends, do we call recycle when setting the threshold?

---

packages/usdc_migration/src/usdc_migration.cairo line 116 at r1 (raw file):

Previously, noa-starkware wrote…

where is the unit size?

l1_transfer_unit? or do you mean the list?
This PR is not ready, which is why I left it as a draft, I wanted to wait until set_threshold is merged before going forward.

[remollemo]

@remollemo reviewed 1 of 3 files at r2.
Reviewable status: 1 of 6 files reviewed, 8 unresolved discussions (waiting on @arad-starkware and @noa-starkware)

---

packages/usdc_migration/src/starkgate_interface.cairo line 4 at r4 (raw file):

#[starknet::interface]
pub(crate) trait ITokenBridge<TContractState> {

does it have to be pub?

Code quote:

pub(crate) 

---

packages/usdc_migration/src/usdc_migration.cairo line 44 at r4 (raw file):

        l1_transfer_unit: u256,
        /// L1 USDC token address.
        l1_usdc_token_address: EthAddress,

Suggestion:

l1_token_address

---

packages/usdc_migration/src/interface.cairo line 5 at r4 (raw file):

    /// Exchange `amount` of legacy token for new token.
    /// Precondition: Caller has approved the contract to spend `amount` of legacy token.
    fn exchange_legacy_for_new(ref self: T, amount: u256);

Suggestion:

  fn exchange_legacy_for_new(ref self: T, amount: u256);
  fn exchange_legacy_for_new(ref self: T, amount: u256);

---

packages/usdc_migration/src/interface.cairo line 6 at r4 (raw file):

    /// Precondition: Caller has approved the contract to spend `amount` of legacy token.
    fn exchange_legacy_for_new(ref self: T, amount: u256);
    fn recycle(ref self: T);

we don't recycle, we send legacy tokens to L1.

Code quote:

    fn recycle(ref self: T);

[noa-starkware]

Reviewable status: 1 of 6 files reviewed, 7 unresolved discussions (waiting on @arad-starkware)

---

packages/usdc_migration/src/usdc_migration.cairo line 86 at r1 (raw file):

Previously, arad-starkware wrote…

depends, do we call recycle when setting the threshold?

We can do that if we want to

[noa-starkware]

Reviewable status: 0 of 13 files reviewed, 17 unresolved discussions (waiting on @arad-starkware and @remollemo)

---

packages/usdc_migration/src/usdc_migration.cairo line 86 at r1 (raw file):

Previously, noa-starkware wrote…

We can do that if we want to

Do we want to?

---

packages/usdc_migration/src/usdc_migration.cairo line 41 at r7 (raw file):

        l1_recipient: EthAddress,
        /// Address in L2 that gets the remaining USDC.
        owner_l2_address: ContractAddress,

revert

Code quote:

        /// Address in L2 that gets the remaining USDC.
        owner_l2_address: ContractAddress,

---

packages/usdc_migration/src/usdc_migration.cairo line 50 at r7 (raw file):

        batch_size: u256,
        /// L1 USDC token address.
        l1_token_address: EthAddress,

add it next to the other addresses in storage

Code quote:

        /// L1 USDC token address.
        l1_token_address: EthAddress,

---

packages/usdc_migration/src/usdc_migration.cairo line 71 at r7 (raw file):

        starkgate_address: ContractAddress,
        legacy_threshold: u256,
        l1_token_address: EthAddress,

Suggestion:

        ref self: ContractState,
        legacy_token: ContractAddress,
        new_token: ContractAddress,
        l1_token_address: EthAddress,
        l1_recipient: EthAddress,
        owner: ContractAddress,
        starkgate_address: ContractAddress,
        legacy_threshold: u256,

---

packages/usdc_migration/src/usdc_migration.cairo line 84 at r7 (raw file):

        self.batch_size.write(LARGE_BATCH_SIZE);
        self.ownable.initializer(:owner);
        self.l1_token_address.write(l1_token_address);

move

---

packages/usdc_migration/src/usdc_migration.cairo line 103 at r7 (raw file):

    pub impl USDCMigrationImpl of IUSDCMigration<ContractState> { //impl logic
        fn swap_to_new(ref self: ContractState, amount: u256) {
            let contract_address = get_contract_address();

What is the cost of this line? @remollemo

---

packages/usdc_migration/src/usdc_migration.cairo line 122 at r7 (raw file):

                    to_token: self.legacy_token_dispatcher.read(),
                    :amount,
                );

Suggestion:

self
                ._swap(
                    contract_address: get_contract_address(),
                    from_token: self.new_token_dispatcher.read(),
                    to_token: self.legacy_token_dispatcher.read(),
                    :amount,
                );

---

packages/usdc_migration/src/usdc_migration.cairo line 128 at r7 (raw file):

            let contract_address = get_contract_address();
            self._send_to_l1(:contract_address);
        }

IMO delete

Code quote:

        fn send_to_l1(ref self: ContractState) {
            let contract_address = get_contract_address();
            self._send_to_l1(:contract_address);
        }

---

packages/usdc_migration/src/usdc_migration.cairo line 174 at r7 (raw file):

    #[generate_trait]
    impl InternalUSDCMigration of InternalUSDCMigrationTrait {

why?

---

packages/usdc_migration/src/usdc_migration.cairo line 198 at r7 (raw file):

        fn _send_legacy_to_l1(self: @ContractState, amount: u256) {
            self.send_units_to_l1(batch_size: amount, batch_count: 1);

rename

---

packages/usdc_migration/src/usdc_migration.cairo line 201 at r7 (raw file):

        }

        fn _send_to_l1(ref self: ContractState, contract_address: ContractAddress) {

Why not do the for inside this function and calls _send_legacy_to_l1 just gets amount and invokes initiate_token_withdraw? Now there are too many functions that handle the same.

---

packages/usdc_migration/src/usdc_migration.cairo line 225 at r7 (raw file):

                starkgate_dispatcher
                    .initiate_token_withdraw(:l1_token, :l1_recipient, amount: batch_size);
            }

Or use while instead of calculate and then use for.
You can pass also l1_token, l1_recipient, starkgate_bridge to save readings.

Suggestion:

        fn _send_legacy_to_l1(self: @ContractState, amount: u256) {
            starkgate_dispatcher
                    .initiate_token_withdraw(l1_token, l1_recipient, amount);
        }

        fn _send_to_l1(ref self: ContractState, contract_address: ContractAddress) {
            let legacy_balance = self
                .legacy_token_dispatcher
                .read()
                .balance_of(account: contract_address);
            if legacy_balance < self.legacy_threshold.read() {
                return;
            }

            let batch_size = self.batch_size.read();
            let batch_count = legacy_balance / batch_size;
            for _ in 0..batch_count {
                self._send_legacy_to_l1(amount: batch_size)

            self.emit(SentToL1 { amount: batch_size * batch_count, batch_size, batch_count });
        }

---

packages/usdc_migration/src/interface.cairo line 10 at r7 (raw file):

    fn swap_to_legacy(ref self: T, amount: u256);
    /// Sends legacy tokens to L1 in batches of `l1_transfer_unit`.
    fn send_to_l1(ref self: T);

I think we dont need that function

[arad-starkware]

Reviewable status: 0 of 13 files reviewed, 17 unresolved discussions (waiting on @arad-starkware, @noa-starkware, and @remollemo)

---

packages/usdc_migration/src/interface.cairo line 6 at r4 (raw file):

Previously, remollemo wrote…

we don't recycle, we send legacy tokens to L1.

Done.

---

packages/usdc_migration/src/interface.cairo line 10 at r7 (raw file):

Previously, noa-starkware wrote…

I think we dont need that function

Done.

---

packages/usdc_migration/src/starkgate_interface.cairo line 4 at r4 (raw file):

Previously, remollemo wrote…

does it have to be pub?

so we use it in the impl yes, should i maybe duplicate this interface for the test so we don't have to do this?

---

packages/usdc_migration/src/usdc_migration.cairo line 86 at r1 (raw file):

Previously, noa-starkware wrote…

Do we want to?

yes

---

packages/usdc_migration/src/usdc_migration.cairo line 41 at r7 (raw file):

Previously, noa-starkware wrote…

revert

Done.

---

packages/usdc_migration/src/usdc_migration.cairo line 50 at r7 (raw file):

Previously, noa-starkware wrote…

add it next to the other addresses in storage

Done.

---

packages/usdc_migration/src/usdc_migration.cairo line 84 at r7 (raw file):

Previously, noa-starkware wrote…

move

Done.

---

packages/usdc_migration/src/usdc_migration.cairo line 103 at r7 (raw file):

Previously, noa-starkware wrote…

What is the cost of this line? @remollemo

I removed it... it's ridiculous...

---

packages/usdc_migration/src/usdc_migration.cairo line 128 at r7 (raw file):

Previously, noa-starkware wrote…

IMO delete

Done.

---

packages/usdc_migration/src/usdc_migration.cairo line 174 at r7 (raw file):

Previously, noa-starkware wrote…

why?

that's what i called it before your PRs were merged and I forgot to change it

---

packages/usdc_migration/src/usdc_migration.cairo line 198 at r7 (raw file):

Previously, noa-starkware wrote…

rename

Done.

---

packages/usdc_migration/src/usdc_migration.cairo line 201 at r7 (raw file):

Previously, noa-starkware wrote…

Why not do the for inside this function and calls _send_legacy_to_l1 just gets amount and invokes initiate_token_withdraw? Now there are too many functions that handle the same.

I'd rather split this since this way _send_legacy_batches_to_l1 is exactly the shared logic between send_legacy_balance_to_l1 and send_legacy_batches_to_l1

---

packages/usdc_migration/src/usdc_migration.cairo line 225 at r7 (raw file):

Previously, noa-starkware wrote…

Or use while instead of calculate and then use for.
You can pass also l1_token, l1_recipient, starkgate_bridge to save readings.

I want the calculation for the event anyway, and as I said above, this way I get to reuse logic between the functions that send to l1 without reading from storage in both.

---

packages/usdc_migration/src/usdc_migration.cairo line 71 at r7 (raw file):

        starkgate_address: ContractAddress,
        legacy_threshold: u256,
        l1_token_address: EthAddress,

Done.

---

packages/usdc_migration/src/usdc_migration.cairo line 122 at r7 (raw file):

                    to_token: self.legacy_token_dispatcher.read(),
                    :amount,
                );

Done.

---

packages/usdc_migration/src/usdc_migration.cairo line 44 at r4 (raw file):

        l1_transfer_unit: u256,
        /// L1 USDC token address.
        l1_usdc_token_address: EthAddress,

Done.

[remollemo]

Reviewable status: 0 of 13 files reviewed, 17 unresolved discussions (waiting on @arad-starkware and @noa-starkware)

---

packages/usdc_migration/src/usdc_migration.cairo line 67 at r9 (raw file):

        new_token: ContractAddress,
        l1_recipient: EthAddress,
        l1_token_address: EthAddress,

why should we get l1_token_address in constructor?
we need it only to push stuff thru SG to L1,
we can very well just use the ITokenBridge::get_l1_token(legacy_token) either every time or to store it in construction using this.
the only "agreeable" reason IMO to get it is to validate the wiring. but you have no wiring validatoin to begin with.

Code quote:

    l1_token_address: EthAddress,

---

packages/usdc_migration/src/usdc_migration.cairo line 186 at r9 (raw file):

        }

        fn send_legacy_batches_to_l1(ref self: ContractState) {

the batchkiada is internal. the highlevel action is sending them tokens to l1.

Suggestion:

send_legacy_tokens_to_l1