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

RemoteTreasury pallet #32

Closed
1 task done
dutterbutter opened this issue Apr 7, 2021 · 6 comments
Closed
1 task done

RemoteTreasury pallet #32

dutterbutter opened this issue Apr 7, 2021 · 6 comments
Labels
epic p4 Low Priority

Comments

@dutterbutter
Copy link
Contributor

dutterbutter commented Apr 7, 2021

Implementation details

Similar to LocalTreasury with extra functionality to control assets on other chains via XCMP. The RemoteTreasury will be responsible for DOT, which will be controlled through XCMP on the relay chain.

Acceptance Criteria

The completed pallet should have the following functionality:

  • execute
@dutterbutter dutterbutter added p4 Low Priority epic labels Apr 7, 2021
@mattsse
Copy link
Contributor

mattsse commented Apr 8, 2021

@mattsse
Copy link
Contributor

mattsse commented Apr 12, 2021

From the design document it was not immediately obvious to me which functionality the remote treasury should provide as extrinsic.

The RemoteTreasury will be responsible for DOT, which will be controlled through XCMP on the relay chain.

However, only dispatching calls via execute is listed here.
My understanding is, the remote-treasury will manage the DOTs deposited on the relay chain and, similar to local-treasury, the admin should be able to withdraw, or rather transfer, DOTs from this treasury's account into a recipient account on the relay chain via XCMP.

I attempted to implement this in #34 as transfer_dot extrinsic, where the admin can initiate a transfer of DOT by executing issuing a withdrawal from the treasury account on the relay that is then deposited into the recipient's account on the relay chain, via XCMP.

I'm currently investigating how to properly test this, but need to clarify that this is the actual anticipated functionality of the pallet. @willemolding @ansermino

@ansermino
Copy link
Member

When writing the design doc it was unclear how exactly controlling DOT on the relay chain would function. To compensate for this, the generalized functionality of dispatching a call on another parachain seemed sufficient to support the needs of the treasury. Hence the execute method was defined as a way to dispatch a call to a predefined destination.

Now that XCMP is coming together, I think it's worth rethinking this. What you've implemented in #34 seems sensible, although I do have one gripe. If possible, it would be nice not to constrain the pallet to interact only with DOT and only on the relay chain. I think this would simply require these vars to be passed into the config:

let asset = MultiAsset::ConcreteFungible {
id: Junction::Parent.into(),
amount: amount.into(),
};
// the recipient's account on the relay chain
let dest = (
Junction::Parent,
Junction::AccountId32 {
network: T::RelayChainNetworkId::get(),
id: T::AccountId32Convert::convert(recipient.clone()),
},
)
.into();

@mattsse
Copy link
Contributor

mattsse commented Apr 13, 2021

I also think that it is important to implement this in a more general way.
They way this currently seems to work is by introducing a currency identifier and declare (id => location) pairs for the trusted reserves for example CurrencyId::DOT => Junction::Parent.

The orml-xtokens-pallet seems to provide this already.

@mattsse
Copy link
Contributor

mattsse commented Apr 13, 2021

@ansermino I've added support for general transfers in #34.
I have added support for general transfers, depending on the location of the asset and the destination different routines are executed by xcm. I have tried to a concise description of the possible scenarios:

https://github.com/ChainSafe/PINT/pull/34/files#diff-dff9576416b319287b7ee69b44e99ee4f78b9cc4725d8e3889b7cb387759cb3cR138-R164

@dutterbutter
Copy link
Contributor Author

Closing, no longer required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic p4 Low Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants