Skip to content

Bring back NativeToForeign storage in EthereumSystem to be more resilient against changes#9901

Draft
girazoki wants to merge 1 commit intoparitytech:masterfrom
girazoki:girazoki-bring-back-nativeToForeign
Draft

Bring back NativeToForeign storage in EthereumSystem to be more resilient against changes#9901
girazoki wants to merge 1 commit intoparitytech:masterfrom
girazoki:girazoki-bring-back-nativeToForeign

Conversation

@girazoki
Copy link
Contributor

@girazoki girazoki commented Oct 1, 2025

This PR brings back the storage removed in #8473. The motivation on that PR was that the storage was not used, and it was true. However I think it should be used to have a more resilient structure against potential breaking changes in the future. The rationale is the following:

A) If any of the HashedDescription or xcm locations changes there is a quite complex migration to be performed without this PR. It would involve both migrating bridgehub storages and ethereum mappings in a coordinated manner.

B) With this new PR, there is no downside besides having to store a few more bytes in EthereumSystem. The exporter has exactly the same amount of reads (as we were already checking whether the asset existed in EthereumSystem). This PR maintains the HashedDescription implementation when the asset is registered, but from there on, it relies on EthereumSystem to do the conversion Location->TokenId. Now any of the above breaking changes does not imply a migration, as the hash would still match the old one.

I am making this PR as draft as I want to gather feedback. Obviously there are a few TODOs yet that I will implement if the feedback is positive

Checklist

  • Migration to re-populate NativeToForeign
  • Adapt tests
  • Adapt benchmarks (if needed)

@yrong
Copy link
Contributor

yrong commented Oct 13, 2025

@girazoki I’m fine with this change, though I’m not fully convinced it’s necessary — a breaking change in HashedDescription seems unlikely, and there are broader incompatibility cases to consider beyond the current one.

For instance, cross chain contract calls follows the same pattern, requiring an Agent to be created for a specific location (as you already mentioned in the telegram channel). So if such a case does occur, I’d prefer addressing it with a one-time migration rather than adding extra code or storage.

@girazoki
Copy link
Contributor Author

girazoki commented Oct 14, 2025

@girazoki I’m fine with this change, though I’m not fully convinced it’s necessary — a breaking change in HashedDescription seems unlikely, and there are broader incompatibility cases to consider beyond the current one.

For instance, cross chain contract calls follows the same pattern, requiring an Agent to be created for a specific location (as you already mentioned in the telegram channel). So if such a case does occur, I’d prefer addressing it with a one-time migration rather than adding extra code or storage.

I do believe contract calls are not that hard to migrate. I dont believe those agent-created contracts will contain a lot of tokens so the loss of simply deprecating the old one and using the new ones might not be too bad. also this might require a user-level only migration.

although I partially agree with you, this also covers the case in which I want to express with more detail my token. for instance, maybe at the beginning I had my token expressed as Location { parents:0, interior: X1(GeneralIndex(0)) } and I want to migrate it to a location of the form: { parents:0, interior: X2(PalletInstance(x), GeneralIndex(0)) }. this also happened in the past in assethub and other projects. A simple migration in bridgehub should suffice to cover this case

In short I agree its not strictly necessary being optimistic. but you also dont win anything besides not storing a few bytes by removing that storage item.

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.

2 participants