kkrt-labs / kakarot-ssj

Kakarot zkEVM - rewrite in the latest version of Cairo
https://www.kakarot.org
MIT License
131 stars 69 forks source link

feat: implement 0x41 - COINBASE Opcode #150

Closed Eikix closed 11 months ago

Eikix commented 1 year ago
SinceGroup
FrontierBlock Information

Index 1 is top of the stack. See PUSH.

Stack output

  1. address: miner's 20-byte address.

Example

InputOutput
10x5B38Da6a701c568545dCfcB03FcB875f56beddC4

Error cases

The state changes done by the current context are reverted in those cases:

  • Not enough gas.
  • Stack overflow.
TAdev0 commented 1 year ago

@Eikix can i implement this opcode?

Eikix commented 1 year ago

I'm still wondering what should be the behaviour for this opcode. Since coinbase is the rewarded miner/validator of the block, here it should be the starknet sequencer, but it has a 31 byte address,

TAdev0 commented 1 year ago

couldn't we modify the output of the opcode to store the entire starknet sequencer address in a uint256, and decide of a 0 arbitrary value for the first 2 bytes?

Eikix commented 1 year ago

I'd think modifying the output of an opcode breaks compat, we'd rather never break compat, that being said, it's true that a stack output is always 32 bytes. but still, since its an address as an output, we'd expect the first 12 bytes to be 0 so idk

@enitrat wdyt

lambda-0x commented 1 year ago

how about returning a special EthAddress which is a eth smart contract, so every payment to COINBASE address goes to that special ethereum account, and kakarot SC at end of every execution forward whatever there is in that address to current sequencer (this way even changing who this token goes to is easy)

Eikix commented 1 year ago

but philosophically, coinbase receives a block reward afaik, so it should be the chain sequencer when someone uses kakarot, they're not paying a fee (yet) to any coinbase address

so that special EthAddress would just always be at 0 balance

lambda-0x commented 1 year ago

for now internally Kakarot doesn't charge fees and there is no block reward so there won't be any balance related to that, but what about someone that send token directly to COINBASE address via .call?

in practice there won't be any need to do so, but for compatibility we need to allow it.

Eikix commented 1 year ago

yes good thinking!

so we need somehow to be able to link an eth-address (20 bytes) and the sequencer address (31 bytes)

there are a few alternatives, some more tedious than others, in Kakarot cairo zero, we push a 31 bytes addr onto the stack, not sure this is right https://github.com/kkrt-labs/kakarot/blob/0c6e7a62a38e973ad6ceebfbf648841749ffe0ee/src/kakarot/instructions/block_information.cairo#L100 https://github.com/kkrt-labs/kakarot/blob/0c6e7a62a38e973ad6ceebfbf648841749ffe0ee/src/kakarot/constants.cairo#L41

@enitrat @ClementWalter any opinion on this?

enitrat commented 1 year ago

I think the question is more philosophical and can be rephrased to "who should COINBASE be" - do we want it to be the sequencer address? If so, we need to compute the sequencer's associated EVM address on KKT. Do we want it to be the Kakarot wallets directly? Or something else?

In my opinion, it should be a wallet that someone has control over, in case we need to retrieve lost funds at one point or another. So either we deploy the sequencer wallet (and give it to SW?) or we just create a wallet specifically for the COINBASE opcode. I would go with option 2.

Eikix commented 1 year ago

But philosophically, the coinbase address is the block proposer, no? I think as long as Starknet has a model where the sequencer receives the fee (in max_fee and actual_fee), coinbase should be pointing to an associated address that being said, there is no EVM equivalent to that address atm, so it may prove difficult

I'm not against deploying once a coinbase account, but it won't be the actual acct that is able to levy fees on txs

enitrat commented 1 year ago

but it won't be the actual acct that is able to levy fees on txs

I think that's a reasonable tradeoff, but the problem will occur again when we have decentralized sequencers. How will we proceed then? Unless there is a starknet syscall to get the block proposer we'll be stuck again and will need a solution that doesn't really match

lambda-0x commented 1 year ago

But philosophically, the coinbase address is the block proposer, no

yes but block proposer would already get paid for including the chain's native transaction in the block. This two layer reward for the sequencer might get complicated.

One case i could think of is incase of chain with decentralized sequencer and KKT domination % of gas spend on chain, it might be in sequencers interest to reorder block in such a way that its not good for user. (just thinking about random cases which might need consideration, i am not sure if this is an issue)

IMO whatever value is accrued due to COINBASE is value generated by KKT, so i think it belongs to KKT. Team (in future DAO maybe?) should be able to make decision what to do with it.

enitrat commented 1 year ago

agreed !

Eikix commented 1 year ago

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 link everytime you take an issue and check your issue isn't targeted by a known bug.

Eikix commented 1 year ago

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.

Eikix commented 11 months ago

I think we have four options for coinbase:

Eikix commented 11 months ago

new idea: Let's set the sequencer_address as we control the full node wrapping Kakarot

#[derive(Copy, Drop)]
struct ExecutionInfo {
    block_info: Box<BlockInfo>,
    tx_info: Box<TxInfo>,
    caller_address: ContractAddress,
    contract_address: ContractAddress,
    entry_point_selector: felt252,
}

#[derive(Copy, Drop, Serde)]
struct BlockInfo {
    block_number: u64,
    block_timestamp: u64,
    sequencer_address: ContractAddress,
}

exec_coinbase:

  1. get_execution_info_v2_syscall
  2. get sequencer_address for execution info
  3. Important choice: try to convert this contract address to an EVM address -> force ContractAddress < 2**160. Which means we "override" the type system and consider this address to be an EVM addr
  4. return
Eikix commented 11 months ago

new idea: Let's set the sequencer_address as we control the full node wrapping Kakarot

#[derive(Copy, Drop)]
struct ExecutionInfo {
    block_info: Box<BlockInfo>,
    tx_info: Box<TxInfo>,
    caller_address: ContractAddress,
    contract_address: ContractAddress,
    entry_point_selector: felt252,
}

#[derive(Copy, Drop, Serde)]
struct BlockInfo {
    block_number: u64,
    block_timestamp: u64,
    sequencer_address: ContractAddress,
}

exec_coinbase:

  1. get_execution_info_v2_syscall
  2. get sequencer_address for execution info
  3. Important choice: try to convert this contract address to an EVM address -> force ContractAddress < 2**160. Which means we "override" the type system and consider this address to be an EVM addr
  4. return

https://github.com/keep-starknet-strange/madara/blob/6b369880ea7ebd574abf7860286fa6c93c973249/crates/pallets/starknet/src/lib.rs#L451