Skip to content

Conversation

@franciscoaguirre
Copy link
Contributor

No description provided.

The new instruction looks like this:

```rust
Publish { data: BoundedVec<Key, Value, Bound> }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Publish { data: BoundedVec<Key, Value, Bound> }
Publish { data: BoundedVec<(Key, Value), Bound> }


- Should we account for the published data's size via Weight?
- Should we introduce a new instruction for storage deposits or add another operand to the `Publish` instruction specifying the assets used?
- Should there be a way to delete the published data via XCM?
Copy link
Contributor

Choose a reason for hiding this comment

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

It is an implementation detail, but we could use the approach of updating a value by using the same key with a new value, and deleting a value by using the same key with as value an empty Vec.

Publish { data: BoundedVec<Key, Value, Bound> }
```

`Key` and `Value` are arrays of bits, they could be anything.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe clearly specify that Key and Value are both Vec<U8>

```rust
WithdrawAsset(/* ... */),
BuyExecution { /* ... */ }
Publish { data: [(b"My key", b"My value")] }
Copy link

Choose a reason for hiding this comment

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

What is the origin of this data? Can that be stored along with the data itself? Or is this operation by default restricted to a specific origin? E.g., I am a parachain interested in this data but only if it comes from another parachain, not from users within it.

Copy link

Choose a reason for hiding this comment

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

If there's some additional logic as to who can send/execute this instruction, it should be added here.

The new instruction looks like this:

```rust
Publish { data: BoundedVec<Key, Value, Bound> }
Copy link

Choose a reason for hiding this comment

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

Could different chains have different values set for Bound? And if so, the sending chain should be aware of the Bound of the receiving chain, right?


## Questions and open Discussions (optional)

- Should we account for the published data's size via Weight?
Copy link

Choose a reason for hiding this comment

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

I believe it should be up to the implementation on the receiving chain, don't you think? Basically, it would depend on what that chain does with that data and its size. For example, if this instruction triggers an extrinsic whose execution time depends on the size of the data, you should BuyExecution that is enough for that size of data.

Copy link

Choose a reason for hiding this comment

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

Which leads me to... would chains be able to configure an Executor or Dispatcher to handle this kind of instruction?

## Questions and open Discussions (optional)

- Should we account for the published data's size via Weight?
- Should we introduce a new instruction for storage deposits or add another operand to the `Publish` instruction specifying the assets used?
Copy link

Choose a reason for hiding this comment

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

If chains are able to configure the way this instruction is processed, I think this could be used for storage deposits. For example in storage chains, it could express the intention of storing a file identified by a hash (and some other metadata needed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants