-
Notifications
You must be signed in to change notification settings - Fork 2
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
Polkadot v0.9.43 #69
Polkadot v0.9.43 #69
Conversation
cc @mustermeiszer rebased this branch on latest master to ensure that we get the latest changes in here as well. |
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.
LGTM! Thanks for working on this! 🙌🏻
core/src/builder/parachain.rs
Outdated
.map_err(|e| { | ||
tracing::error!( | ||
target: DEFAULT_COLLATOR_LOG_TARGET, | ||
error = ?e, | ||
"Could not cast upward messages.", | ||
) | ||
}) | ||
.ok()?; |
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'm asking here, but the question goes more for all these patterns. If fudge
intention is to be used for testing. Why, instead of tracing and propagating a None, do we no just unwrap everywhere? That way, when it doesn't work, the error just panic with the correct message in the correct line which generate it.
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 @mustermeiszer can provide a better answer to this ^^.
In my fudge branch for xcm tests I'm addressing some of these and propagating some more errors in cases similar to 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.
You might not want to use it soley for testing. You can also use it for other things like forking a network, etc. I think unwrapping in a dependency library is not really an option IMO.
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.
unwrapping in a dependency library is not really an option IMO.
Ok, this makes sense 👍🏻
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.
Looks already good.
…expose parachain ID from builder
…in collation error
Latest changes currently being tested here as well. Keeping this PR as a drat until we confirm that tests work as expected and no other changes are needed. |
55e522d
to
bf29497
Compare
bf29497
to
c779b90
Compare
2354922
to
a7162ca
Compare
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.
Sick, sick, sick!
Nice that you got that one working!
let call = PRuntimeCall::System(frame_system::Call::remark_with_event { | ||
remark: remark.clone(), | ||
}); | ||
|
||
let instruction: Instruction<PRuntimeCall> = Transact { | ||
origin_kind: OriginKind::SovereignAccount, | ||
require_weight_at_most: Weight::from_parts(200_000_000, 0), | ||
call: call.encode().into(), |
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.
SICK!
@mustermeiszer can we go ahead and merge this and keep the branch? |
Adding support for
polkadot-v0.9.43
.