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

May need another APIs to get the proofs of Axon's system contracts #1561

Open
yangby-cryptape opened this issue Nov 15, 2023 · 7 comments
Open
Labels
document enhancement New feature or request P-Low Priority t:feature

Comments

@yangby-cryptape
Copy link
Collaborator

yangby-cryptape commented Nov 15, 2023

Description

If I'm correct, data of system contracts could NOT be accessed or proved.

Application Scenarios

  • You can cheat users that data in metadata is not existed, even it really exists.
  • You can NOT prove data in metadata is real.
@yangby-cryptape yangby-cryptape changed the title Data of system contracts could NOT be got or proved. Data of system contracts could NOT be accessed or proved. Nov 15, 2023
@yangby-cryptape yangby-cryptape changed the title Data of system contracts could NOT be accessed or proved. JSON-RPC APIs return incorrect results for system contracts. Nov 15, 2023
@Flouse Flouse added the agenda label Nov 15, 2023
@KaoImin
Copy link
Contributor

KaoImin commented Nov 16, 2023

As the annotation below:

/// The metadata store does not follow the storage layout of EVM smart contract.
/// It use MPT called Metadata MPT with the following layout:
/// | key | value |
/// | -------------------- | ------------------------------------ |
/// | EPOCH_SEGMENT_KEY | `EpochSegment.encode()` |
/// | CKB_RELATED_INFO_KEY | `CkbRelatedInfo.encode()` |
/// | HARDFORK_KEY | `HardforkInfo.encode()` |
/// | epoch_0.be_bytes() | `Metadata.encode()` |
/// | epoch_1.be_bytes() | `Metadata.encode()` |
/// | CONSENSUS_CONFIG | `version + ConsensesConfig.encode()` |
/// | ... | ... |
///
/// All these data are stored in a the `c9` column family of RocksDB, and the
/// root of the Metadata MPT is stored in the storage MPT of the metadata
/// contract Account as follow:
///
/// **Metadata Account**
/// | address | `0xFFfffFFfFFfffFfFffFFfFfFfFffFfffFFFFFf01`|
/// | nonce | `0x0` |
/// | balance | `0x0` |
/// | storage | `storage_root` |
///
/// **Metadata Storage MPT**
/// | METADATA_ROOT_KEY | Metadata MPT root |

The metadata info stores in a column of RocksDB which is different from the trie. However, the metadata trie root is stored in the storage account with the following key-value pair.
/// **Metadata Storage MPT**
/// | METADATA_ROOT_KEY | Metadata MPT root |

pub static ref METADATA_ROOT_KEY: H256 = Hasher::digest("metadata_root");

Besides, the key is a fixed string. So the proof can prove the metadata.

@yangby-cryptape
Copy link
Collaborator Author

yangby-cryptape commented Nov 16, 2023

Besides, the key is a fixed string. So the proof can prove the metadata. ...

Maybe I was Wrong

But you have just spoken your words, and didn't answer my question.

We are not that to compare whose words are more here.

  • But data of system contracts are stored in another column family.

I already said they were in different column families.

The metadata info stores in a column of RocksDB which is different from the trie. However, the metadata trie root is stored in the storage account with the following key-value pair.

What's different between your words and my words?

I think you just repeated what I said.

Let me re-explain my confusion again.

  • If you have clicked the piece of code that I pasted in the issue description, you will see:

    let state_mpt_tree = MPTTrie::from_root(state_root, Arc::clone(&self.trie_db))?;
    let raw_account = state_mpt_tree
    .get(address.as_bytes())?
    .ok_or_else(|| APIError::Adapter("Can't find this address".to_string()))?;
    let account = Account::decode(raw_account).unwrap();
    let storage_mpt_tree = MPTTrie::from_root(account.storage_root, Arc::clone(&self.trie_db))?;

  • As you said above:

    The metadata info stores in a column of RocksDB which is different from the trie.

    Why state_mpt_tree and storage_mpt_tree are loaded from same trie self.trie_db?

    • Can users access the data and their proofs for system contracts from self.trie_db?

      storage_mpt_tree
      .get(hash.as_bytes())?
      .map(Into::into)
      .ok_or_else(|| APIError::Adapter("Can't find this position".to_string()).into())

      value: storage_mpt_tree
      .get(hash.as_bytes())?
      .map(|v| H256::from_slice(&v))
      .ok_or_else(|| {
      Into::<ProtocolError>::into(APIError::Adapter(
      "Can't find this position".to_string(),
      ))
      })?,

@KaoImin
Copy link
Contributor

KaoImin commented Nov 16, 2023

As I said in the previous comment, there is no doubt that we can prove the metadata_root is in the storage trie (through the proof of METADATA_ROOT_KEY). Now the question is how to prove a metadata contains in the metadata trie. 

You're right. It's impossible to get a metadata proof through the eth_getProof API now. Because the metadata is not directly stored in the account storage trie DB.

In order to do this, Axon need another axon_getMetadataProof JsonRPC API. With this interface, user can get the proof of a metadata and prove the metadata contains in the metadata trie, and then prove the metadata root is in the storage trie.

@yangby-cryptape
Copy link
Collaborator Author

yangby-cryptape commented Nov 16, 2023

According to the previous comment, I think there are several new tasks:

  • Documentation for above differences with Ethereum compatibility.
  • New JSON RPC APIs.
    I use metadata contract as the example in issue body, but I think how to prove data in ckb_light_client contract or image_cell contract is more important.
  • Returns errors with useful error message when users call eth_getProof or eth_getStorgeAt on incompatible contracts.
    Axon could point the correct APIs to users. (ref: the next comment).

p.s. I searched the discord channels, public GitHub repositories, and private GitHub repositories, didn't find any content of axon_getMetadataProof. If developers don't write, how could users know.

ping @Flouse

@yangby-cryptape
Copy link
Collaborator Author

yangby-cryptape commented Nov 16, 2023

Will there be more APIS or just two?

In order to do this, Axon need another axon_getMetadataProof JsonRPC API. ...

Since axon_getMetadataProof is only for metadata contract according to its name, but ckb_light_client contract and image_cell contract have same problems.

  • axon_getMetadataStorageAt
  • axon_getMetadataProof
  • axon_getCkbLightClientStorageAt
  • axon_getCkbLightClientProof (p.s. I didn't find any API for it.)
  • axon_getImageCellStorageAt
  • axon_getImageCellProof (p.s. Since contract address is not the same, account proof will be different. The code could be reused.)

@KaoImin
Copy link
Contributor

KaoImin commented Nov 16, 2023

  • axon_getMetadataStorageAt
  • axon_getMetadataProof
  • axon_getCkbLightClientStorageAt
  • axon_getCkbLightClientProof (p.s. I didn't find any API for it.)
  • axon_getImageCellStorageAt
  • axon_getImageCellProof (p.s. Since contract address is not the same, account proof will be different. The code could be reused.)

I prefer these API, rather than provide two APIs axon_getSystemContractStorageAt and axon_getSystemContractProof.

@KaoImin
Copy link
Contributor

KaoImin commented Nov 16, 2023

  • axon_getMetadataStorageAt
  • axon_getMetadataProof
  • axon_getCkbLightClientStorageAt
  • axon_getCkbLightClientProof (p.s. I didn't find any API for it.)
  • axon_getImageCellStorageAt
  • axon_getImageCellProof (p.s. Since contract address is not the same, account proof will be different. The code could be reused.)

I prefer these API, rather than provide two APIs axon_getSystemContractStorageAt and axon_getSystemContractProof.

To be notice that the ckb light client and the image cell use a same column, so these two contracts should provide one pair interface like axon_getHeaderCellStorageAt and axon_getHeaderCellProof.

@Flouse Flouse changed the title JSON-RPC APIs return incorrect results for system contracts. May need another APIs to get the proofs of Axon's system contracts Nov 17, 2023
@Flouse Flouse added t:feature enhancement New feature or request document and removed agenda labels Nov 17, 2023
@Flouse Flouse added the P-Low Priority label Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
document enhancement New feature or request P-Low Priority t:feature
Projects
None yet
Development

No branches or pull requests

3 participants