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

Dispatchable Call for Swapping Treasury Asset #2939

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Jan 16, 2024

Presenting a dispatchable call executed in the integration test environment, where the treasury's native asset is swapped for another asset on Asset Hub through the asset conversion pallet instance.

The swaps can be split into multiple swaps with retries to implement the DCA model.

The purpose of this PR is to explore and demonstrate the potential for swapping and diversifying treasury assets.

@muharem muharem changed the title Call Template: Treasury swaps assets Dispatchable Call: Treasury swaps assets Jan 19, 2024
@muharem muharem changed the title Dispatchable Call: Treasury swaps assets Dispatchable Call for Swapping Treasury Asset Jan 19, 2024
@muharem muharem added R0-silent Changes should not be mentioned in any release notes T10-tests This PR/Issue is related to tests. labels Jan 19, 2024
Comment on lines +813 to +816
match ensure_signed(origin.clone()) {
Err(e) if T::SpendOrigin::ensure_origin(origin).is_err() => Err(e),
_ => Ok(()),
}?;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This could be a separate function.

Is the reason we try the check for SpendOrigin because the call coming from the XCM in the test is not actually a signed origin, but an XCM origin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.
Might be any pallets origin, not only XCM.

Comment on lines +883 to +890
EnsureSignedBy<
impls::MemberByLocation<
xcm_config::RelayTreasuryVoice,
xcm_config::LocationToAccountId,
AccountId,
>,
AccountId,
>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

XCM Transact instruction to swap treasury funds received with SovereignAccount origin kind, because swap calls must be signed by AccountId. The swap calls are not executed with Transact instruction, but scheduled with the schedule call, which is permissioned with this ScheduleOrigin type. This is why we need to EnsureSignedBy here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively same can be achieved if a call requiring an account will be wrapped with a call like dispatch_as_account: #4197

@muharem muharem marked this pull request as ready for review March 7, 2024 18:06
@muharem muharem requested a review from a team as a code owner March 7, 2024 18:06
@@ -13,7 +13,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use crate::*;
// use crate::*;
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
// use crate::*;

@paritytech-review-bot paritytech-review-bot bot requested a review from a team May 6, 2024 15:56
Copy link
Contributor

@georgepisaltu georgepisaltu left a comment

Choose a reason for hiding this comment

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

LGTM, my previous comments are all addressed 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T10-tests This PR/Issue is related to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants