-
Notifications
You must be signed in to change notification settings - Fork 105
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
Smoke test on westend #1291
Smoke test on westend #1291
Conversation
…rk/snowbridge into ron/smoke-test-on-westend
name: "transferToPolkadot", | ||
node_args: "--require=dotenv/config", | ||
script: "./dist/src/transfer_to_polkadot.js", | ||
args: "cron" | ||
}, | ||
{ | ||
name: "transferToEthereum", | ||
node_args: "--require=dotenv/config", | ||
script: "./dist/src/transfer_to_ethereum.js", | ||
args: "cron" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Periodically run the transfer tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we know if they fail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we don't wait here. Instead just depend on alarm infrastructure same as we do for production.
I'll run the task every day and we can alarm on nonce not increased for more than 1 day. It will be notified in https://snowfork.slack.com/archives/C07BG558XE1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! ✨
// Fetches heads for each parachain Id filtering out para threads. | ||
func (conn *Connection) FetchParachainHeads(relayChainBlockHash types.Hash) ([]ParaHead, error) { | ||
// Fetch para heads | ||
paraHeads, err := conn.fetchParaHeads(relayChainBlockHash) | ||
if err != nil { | ||
log.WithError(err).Error("Cannot fetch para heads.") | ||
return nil, err | ||
} | ||
|
||
// fetch ids of parachains (not including parathreads) | ||
var parachainIDs []uint32 | ||
parachainsKey, err := types.CreateStorageKey(conn.Metadata(), "Paras", "Parachains", nil, nil) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
_, err = conn.API().RPC.State.GetStorage(parachainsKey, ¶chainIDs, relayChainBlockHash) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
// filter out parathreads | ||
var parachainHeads []ParaHead | ||
for _, v := range parachainIDs { | ||
if head, ok := paraHeads[v]; ok { | ||
parachainHeads = append(parachainHeads, head) | ||
} | ||
} | ||
return parachainHeads, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be great if we can keep this and make the relayer forwards compatible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, though would prefer to address that in #1288
@alistair-singh I would assume the fix in #1288 will also be compatible with polkadot production?
smoketest/tests/send_token.rs
Outdated
@@ -89,7 +89,7 @@ async fn send_token() { | |||
|
|||
assert_eq!(receipt.status.unwrap().as_u64(), 1u64); | |||
|
|||
let wait_for_blocks = 100; | |||
let wait_for_blocks = 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This increase will cause test failures to be super slow. Is it necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's nessessary for testnet/production as the finality latency is about 1 hour there, so we need to wait a bit longer.
Maybe better to load from some env and decrease it for local setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe better to load from some env and decrease it for local setup.
Good idea!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
web/packages/api/src/environment.ts
Outdated
}, | ||
{ | ||
id: "muse", | ||
name: "Muse", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't look like Muse is deployed on Westend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but would prefer to leave it here as placeholder as Muse will migrate to Westend sooner or later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove it as it will appear in the user interface. We can add it when the integration works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
web/packages/api/src/environment.ts
Outdated
}, | ||
{ | ||
id: "muse", | ||
name: "Muse", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should remove it as it will appear in the user interface. We can add it when the integration works.
@@ -8,22 +9,23 @@ import { | |||
} from "@snowbridge/api" | |||
import { WETH9__factory } from "@snowbridge/contract-types" | |||
import { Wallet } from "ethers" | |||
import cron from "node-cron" | |||
|
|||
const monitor = async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to modify this file if we have added the two new files:
https://github.com/Snowfork/snowbridge/pull/1291/files#diff-1029e29210cb968c5341a4a12d25f34ae82529fc48371f1cfedce3f221f30439R1
and
https://github.com/Snowfork/snowbridge/pull/1291/files#diff-96a9d881f4ba07609e35a3cc2fffd6be3edcab8079fae643ef9cc66f299e6474R1
It seems like there is overlap. And if so I rather revert this file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name: "transferToPolkadot", | ||
node_args: "--require=dotenv/config", | ||
script: "./dist/src/transfer_to_polkadot.js", | ||
args: "cron" | ||
}, | ||
{ | ||
name: "transferToEthereum", | ||
node_args: "--require=dotenv/config", | ||
script: "./dist/src/transfer_to_ethereum.js", | ||
args: "cron" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we know if they fail?
web/packages/api/src/environment.ts
Outdated
minimumTransferAmount: 15_000_000_000_000n, | ||
}, | ||
{ | ||
id: "vETH", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets remove vETH until its registered. This case was for bifrost in rococo. They are not in Westend so I do not think we need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* adds paseo UI * remove muse and bump versions * remove muse and bump versions * remove veth token * paseo things * fix subscan urls * revert version * fix versions
* fix wrap around * logs * doesnt have to be in the same period * testing something * fix * adds test and config * writer * fix compilation * remove temp building relayer * comment
…ron/smoke-test-on-westend
af56d9d
into
alistair/fix-mmr-root-validation
* add back in para threads * fix out of bounds error * forward compatible * fix comment * move function back up * move again * Smoke test on westend (#1291) * Initialize for westend * Update beacon checkpoint * add back in para threads * fix out of bounds error * Smoke tests on westend * Load config from env * Cleanup env * Remove penpal code * Cleanup * Update smoke tests * Add westend env * crontab smoke tests * Split as two tests * Fix typo * Remove assets/parachain non-exist * Revert change * Wait config from env * Load interval from env * Add alarm check no transfer * Adds paseo UI (#1276) * adds paseo UI * remove muse and bump versions * remove muse and bump versions * remove veth token * paseo things * fix subscan urls * revert version * fix versions * Update api package * Fix the merge * Remove unused * Fix find on-chain checkpoint (#1294) * fix wrap around * logs * doesnt have to be in the same period * testing something * fix * adds test and config * writer * fix compilation * remove temp building relayer * comment * bump version --------- Co-authored-by: Alistair Singh <[email protected]> Co-authored-by: Clara van Staden <[email protected]> --------- Co-authored-by: Ron <[email protected]> Co-authored-by: Clara van Staden <[email protected]>
Resolves: https://linear.app/snowfork/issue/SNO-1089, https://linear.app/snowfork/issue/SNO-1097