Skip to content
This repository has been archived by the owner on Oct 22, 2024. It is now read-only.

Custom digest item #65

Closed
wants to merge 3 commits into from
Closed

Custom digest item #65

wants to merge 3 commits into from

Conversation

vgeddes
Copy link

@vgeddes vgeddes commented Dec 14, 2023

Companion for Snowfork/snowbridge#1053

Comment on lines 29 to 32
impl Into<DigestItem> for CustomDigestItem {
fn into(self) -> DigestItem {
match self {
// For snowbridge, we sidestep SCALE-encoding of `CustomDigestItem`, and insert the
// merkle root directly into the DigestItem::Other payload. This reduces complexity
// and gas costs on the Ethereum side.
//
// Other light clients can discriminate between custom digest items by checking the
// length of the encoded payload. If the length is greater than 32, then its a digest
// item inserted by some application other than Snowbridge.
CustomDigestItem::Snowbridge(merkle_root) =>
DigestItem::Other(merkle_root.to_fixed_bytes().into()),
}
}
Copy link

@yrong yrong Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not to sidestep here instead include the Enum prefix(0) as discrimination.

Some companioned change required:

  1. solidity
    https://github.com/Snowfork/snowbridge/blob/c03175b813ee97373843de4f70ea10df1977beb7/contracts/src/Verification.sol#L136
    to something like
    if (item.kind == DIGEST_ITEM_OTHER && item.data.length == 33 && commitment == bytes32(item.data[1:]))
    and IMO it may not increase gas costs that much.
  2. parachain relayer
    https://github.com/Snowfork/snowbridge/blob/c03175b813ee97373843de4f70ea10df1977beb7/relayer/relays/parachain/digest_item.go#L11
    Similar changes required.

@claravanstaden claravanstaden changed the base branch from snowbridge-backup to snowbridge December 15, 2023 09:39
@vgeddes
Copy link
Author

vgeddes commented Dec 20, 2023

This is obsolete right, @yrong, can we close it?

@vgeddes vgeddes closed this Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants