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.

Sign up for GitHub

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

Jump to bottom

Statemine reserve location and universal alias #841

Merged
merged 7 commits into from
May 30, 2023
Merged

Statemine reserve location and universal alias #841

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a89f29a
add statemine parachain
alistair-singh May 17, 2023
c79f018
added configuration
alistair-singh May 19, 2023
eaf3697
update cumulus
alistair-singh May 19, 2023
ec1f244
all configurations added
alistair-singh May 22, 2023
5dbbcb8
Revert "add statemine parachain"
alistair-singh May 22, 2023
64937d1
change chain id
alistair-singh May 23, 2023
320a351
remove unused script
alistair-singh May 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 93 additions & 111 deletions core/packages/test/scripts/configure-statemine.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#!/usr/bin/env sh

set -eu

source scripts/xcm-helper.sh

generate_hex_encoded_call_data() {
local type=$1
local endpoint=$2
Expand All @@ -22,109 +23,29 @@ generate_hex_encoded_call_data() {
return $retVal
}

send_governance_transact() {
local relay_url=$1
local relay_chain_seed=$2
local para_id=$3
local hex_encoded_data=$4
local require_weight_at_most_ref_time=$5
local require_weight_at_most_proof_size=$6
echo " calling send_governance_transact:"
echo " relay_url: ${relay_url}"
echo " relay_chain_seed: ${relay_chain_seed}"
echo " para_id: ${para_id}"
echo " hex_encoded_data: ${hex_encoded_data}"
echo " require_weight_at_most_ref_time: ${require_weight_at_most_ref_time}"
echo " require_weight_at_most_proof_size: ${require_weight_at_most_proof_size}"
echo " params:"

local dest=$(jq --null-input \
--arg para_id "$para_id" \
'{ "v3": { "parents": 0, "interior": { "x1": { "parachain": $para_id } } } }')

local message=$(jq --null-input \
--argjson hex_encoded_data $hex_encoded_data \
--arg require_weight_at_most_ref_time "$require_weight_at_most_ref_time" \
--arg require_weight_at_most_proof_size "$require_weight_at_most_proof_size" \
'
{
"v3": [
{
"unpaidexecution": {
"weight_limit": "unlimited"
}
},
{
"transact": {
"origin_kind": "superuser",
"require_weight_at_most": {
"ref_time": $require_weight_at_most_ref_time,
"proof_size": $require_weight_at_most_proof_size,
},
"call": {
"encoded": $hex_encoded_data
}
}
}
]
}
')

echo ""
echo " dest:"
echo "${dest}"
echo ""
echo " message:"
echo "${message}"
echo ""
echo "--------------------------------------------------"

npx polkadot-js-api \
--ws "${relay_url?}" \
--seed "${relay_chain_seed?}" \
--sudo \
tx.xcmPallet.send \
"${dest}" \
"${message}"
}

add_exporter_config() {
local relay_url=$1
local relay_chain_seed=$2
local statemine_para_id=$3
local statemine_para_endpoint=$4
local bridge_hub_para_id=$5
local ethereum_chain_id=$6

echo " calling add_exporter_config:"
echo " relay_url: ${relay_url}"
echo " relay_chain_seed: ${relay_chain_seed}"
echo " statemine_para_id: ${statemine_para_id}"
echo " runtime_para_endpoint: ${statemine_para_endpoint}"
echo " bridge_hub_para_id: ${bridge_hub_para_id}"
echo " ethereum_chain_id: ${ethereum_chain_id}"
echo " params:"

local bridged_network=$(jq --null-input \
--arg ethereum_chain_id "$ethereum_chain_id" \
--arg eth_chain_id "$eth_chain_id" \
'
{
"Ethereum": {
"chainId": $ethereum_chain_id
"chainId": $eth_chain_id
}
}
'
)
# Generate data for Transact (add_exporter_config)
local bridge_config=$(jq --null-input \
--arg bridge_hub_para_id "$bridge_hub_para_id" \
--arg bridgehub_para_id "$bridgehub_para_id" \
--arg bridged_network "$bridged_network" \
'
{
"bridgeLocation": {
"parents": 1,
"interior": {
"X1": { "Parachain": $bridge_hub_para_id }
"X1": { "Parachain": $bridgehub_para_id }
}
},
"allowedTargetLocation": {
Expand All @@ -138,42 +59,103 @@ add_exporter_config() {
}
'
)

echo ""
echo " bridged_network:"
echo "${bridged_network}"
echo ""
echo " bridge_config:"
echo "${bridge_config}"
echo ""
echo "--------------------------------------------------"

local tmp_output_file=$(mktemp)
generate_hex_encoded_call_data \
"add-exporter-config" \
"$statemine_para_endpoint" \
"$statemine_ws_url" \
"$tmp_output_file" \
"$bridged_network" \
"$bridge_config"
local hex_encoded_data=$(cat $tmp_output_file)
rm $tmp_output_file

send_governance_transact "${relay_url}" "${relay_chain_seed}" "${statemine_para_id}" "${hex_encoded_data}" 200000000 12000
send_governance_transact_from_relaychain "${statemine_para_id}" "${hex_encoded_data}" 200000000 12000
}

if [ "$#" -eq 0 ]; then
cat <<EOF
usage:
$0 messages
add_universal_alias() {
echo " calling add_universal_alias:"
local location=$(jq --null-input \
--arg bridgehub_para_id "$bridgehub_para_id" \
'{ "V3": { "parents": 1, "interior": { "X1": { "Parachain": $bridgehub_para_id } } } }') # BridgeHub

messages:
add-exporter-config - adds bridge config for ethereum.
EOF
exit 1
fi
local junction=$(jq --null-input \
--arg eth_chain_id "$eth_chain_id" \
'{ "GlobalConsensus": { "Ethereum": { "chainId": $eth_chain_id } } }')

while [ "$#" -gt 0 ]; do
case "$1" in
add-exporter-config)
shift
add_exporter_config $1 $2 $3 $4 $5 $6
shift 6
;;
*)
echo "Unknown message: $1"
exit 1
;;
esac
done

exit 0
echo ""
echo " location:"
echo "${location}"
echo ""
echo " junction:"
echo "${junction}"
echo ""
echo "--------------------------------------------------"

local tmp_output_file=$(mktemp)
generate_hex_encoded_call_data "add-universal-alias" "${statemine_ws_url}" "${tmp_output_file}" "$location" "$junction"
local hex_encoded_data=$(cat $tmp_output_file)
rm $tmp_output_file

send_governance_transact_from_relaychain "${statemine_para_id}" "${hex_encoded_data}" 200000000 12000
}

add_reserve_location() {
echo " calling add_reserve_location:"

local nativeTokens=$1
# Ethereum Native Tokens contract
local reserve_location=$(jq --null-input \
--arg eth_chain_id "$eth_chain_id" \
--arg nativeTokens "$nativeTokens" \
'{ "V3": {
"parents": 2,
"interior": {
"X2": [
{
"GlobalConsensus": { "Ethereum": { "chainId": $eth_chain_id } },
},
{
"AccountKey20": { "network": { "Ethereum": { "chainId": $eth_chain_id } }, "key": $nativeTokens }
}
]
}
} }')

echo ""
echo " reserve_location:"
echo "${reserve_location}"
echo ""
echo "--------------------------------------------------"

local tmp_output_file=$(mktemp)
generate_hex_encoded_call_data "add-reserve-location" "${statemine_ws_url}" "${tmp_output_file}" "$reserve_location"
local hex_encoded_data=$(cat $tmp_output_file)
rm $tmp_output_file

send_governance_transact_from_relaychain "${statemine_para_id}" "${hex_encoded_data}" 200000000 12000
}

configure_statemine() {
# Allows messages to be sent from Substrate to ETH.
add_exporter_config
# Allow messages to be sent from ETH to Substrate.
add_universal_alias
# Allow NativeTokens contract to ReserveAssetDeposited xcm instruction.
add_reserve_location $(address_for NativeTokens)
Copy link
Collaborator

@vgeddes vgeddes May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I forgot to sync with you about this. The problem is that we can't use the contract address for NativeTokens as the reserve location. If we replace NativeTokens with an upgraded contract, that will break the config on Statemint.

Instead, since the reserve location is largely symbolic, I would just use a dummy address for now.

When BridgeHub forwards XCM messages to Statemint, we can make our router say these messages originated from the dummy address too (rather than from OutboundQueue.sol). Statemint will inspect the origin of incoming messages to ensure they are the owners of the assets in ForeignAssets pallet.

At some point in the next week or two, we can replace the dummy address with the address of a real contract in our stack. This will just be a do-nothing contract, with the only property being that it was deployed by us.

contract Snowbridge {}

Copy link
Contributor Author

@alistair-singh alistair-singh May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking that for upgrades we would just add the new NativeTokens contract address as a reserve location using the same add_reserve_location extrinsic, potentially even in the same call which sends the upgrade message across the bridge. The bridge-transfer pallet can have both versions of the contract added and can be run back to back. Once upgrade is complete we can remove the old contract address using remove_reserve_location extrinsic.

Do you see any problems with this approach?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also use vault contracts as a reserve location as they live forever and actually hold funds. So maybe a better choice than a dummy address.

Copy link
Collaborator

@vgeddes vgeddes May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking that for upgrades we would just add the new NativeTokens contract address as a reserve location using the same add_reserve_location extrinsic, potentially even in the same call which sends the upgrade message across the bridge. The bridge-transfer pallet can have both versions of the contract added and can be run back to back. Once upgrade is complete we can remove the old contract address using remove_reserve_location extrinsic.

Do you see any problems with this approach?

There are various problem with this approach. We can only execute add_reserve_location, etc via governance proposal voted upon by the Polkadot community. It can take weeks to execute a governance proposal.

It makes smart contract upgrades, slower, more brittle. Since it requires two governance proposals: (1) Update the ethereum side, (2) update the polkadot side. Alternatively, if you combine the proposals, then if one subtask fails, you end up with inconsistent state which requires more governance proposals to fix.

Copy link
Collaborator

@vgeddes vgeddes May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We could also use vault contracts as a reserve location as they live forever and actually hold funds. So maybe a better choice than a dummy address.

Vault contracts are an internal implementation detail. I don't want to expose them in our public API, which could cause problems if ever change our design/implementation later down the line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see. I have created this issue to switch to a dummy address.
https://linear.app/snowfork/issue/SNO-450
I think we should defer this for now. While I am focusing on trying to get the round trip transfer done we can continue with using NativeTokens?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It won't work without a dummy address, and we need to update BridgeHub so that the XCM messages it sends to Statemint are seen as coming from a MultiLocation containing the dummy address as a junction.

Because the XCM config in Statemint/Westmint has this constraint:
https://github.com/paritytech/cumulus/pull/2013/files#diff-07287c62dd91150d975f7504bd14cc62d9889c44f7ba8ca44c30c03b05d5b071R383

https://github.com/paritytech/cumulus/blob/c167adde899a1a3818e19cf55dfe9a6b907bff63/parachains/pallets/bridge-transfer/src/impls.rs#L43

It checks that an asset has a multilocation that has the reserve location as a prefix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also see this comment in the bridge transfer pallet: https://github.com/paritytech/cumulus/blob/c167adde899a1a3818e19cf55dfe9a6b907bff63/parachains/pallets/bridge-transfer/src/lib.rs#L42

Copy link
Contributor

Choose a reason for hiding this comment

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

If we replace NativeTokens with an upgraded contract, that will break the config on Statemint.

Seems proxy pattern will fit well into this case, the proxy address of NativeTokens will not change when underlying logic contract upgraded. Another benifit with this pattern is storage isolation so no need for migration when upgrading.

Cons are as Vincent mentioned before a bit brittle and not easy to maintain. #801 (comment)

}

if [ -z "${from_start_services:-}" ]; then
echo "configuring statemine"
configure_statemine
wait
fi
9 changes: 0 additions & 9 deletions core/packages/test/scripts/deploy-polkadot.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,4 @@ zombienet_launch() {

deploy_polkadot() {
zombienet_launch

# Configure statemine
scripts/configure-statemine.sh add-exporter-config \
$relaychain_ws_url \
$relaychain_sudo_seed \
$statemine_para_id \
$statemine_ws_url \
$bridgehub_para_id \
$eth_chain_id
}
5 changes: 3 additions & 2 deletions core/packages/test/scripts/helpers/generateHexEncodedCallData.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,10 @@ async function connect(endpoint, types = {}) {
}

function writeHexEncodedBytesToOutput(method, outputFile) {
console.log("Payload (hex): ", method.toHex());
const hex = method.toHex();
console.log("Payload (hex): ", hex);
console.log("Payload (bytes): ", Array.from(method.toU8a()));
fs.writeFileSync(outputFile, JSON.stringify(Array.from(method.toU8a())));
fs.writeFileSync(outputFile, hex);
}

function remarkWithEvent(endpoint, outputFile) {
Expand Down
36 changes: 0 additions & 36 deletions core/packages/test/scripts/send-xcm.sh
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion core/packages/test/scripts/set-env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ eth_network="${ETH_NETWORK:-localhost}"
eth_endpoint_http="${ETH_RPC_ENDPOINT:-http://127.0.0.1:8545}/${INFURA_PROJECT_ID:-}"
eth_endpoint_ws="${ETH_WS_ENDPOINT:-ws://127.0.0.1:8546}/${INFURA_PROJECT_ID:-}"
eth_gas_limit="${ETH_GAS_LIMIT:-5000000}"
eth_chain_id=1
eth_chain_id=15
eth_fast_mode="${ETH_FAST_MODE:-false}"

beefy_state_file="${BEEFY_STATE_FILE:-$output_dir/beefy-state.json}"
Expand Down
6 changes: 5 additions & 1 deletion core/packages/test/scripts/start-services.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ configure_beefy
source scripts/configure-beacon.sh
configure_beacon

# 5. start relayer
# 5. Configure bridgehub exporter on statemine
source scripts/configure-statemine.sh
configure_statemine

# 6. start relayer
echo "Starting relayers"
source scripts/start-relayer.sh
start_relayer
Expand Down
Loading