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

Fix: CFE Witnessing, use parent block metadata when decoding events #4331

Merged
merged 2 commits into from
Dec 11, 2023

Conversation

j4m1ef0rd
Copy link
Contributor

Pull Request

Closes: PRO-1017

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have updated documentation where appropriate.

Summary

The issue is caused by the state_getRuntimeVersion rpc returning the new runtime version when called at the given block, but the events in that block at encoded with the old runtime.

  • Use parent block hash when getting polkadot runtime version & metadata
  • Added a test that gets the events from the block on polkadot mainnet that had the runtime update in it and caused the error that we seen in the logs. Then parses the events and checks for any failure to decode. This test is set to ignore because it uses an external public endpoint.
  • A few other small changes cleaning up code here and there.

@j4m1ef0rd j4m1ef0rd added the CFE label Dec 8, 2023
@j4m1ef0rd j4m1ef0rd self-assigned this Dec 8, 2023
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Attention: 1313 lines in your changes are missing coverage. Please review.

Comparison is base (e6ad83e) 72% compared to head (c71f8f2) 71%.
Report is 58 commits behind head on main.

Files Patch % Lines
...me/src/migrations/threshold_signature_callbacks.rs 0% 144 Missing ⚠️
state-chain/pallets/cf-pools/src/lib.rs 42% 124 Missing and 8 partials ⚠️
engine/src/eth/retry_rpc.rs 6% 103 Missing ⚠️
engine/src/eth/rpc.rs 0% 85 Missing ⚠️
state-chain/custom-rpc/src/lib.rs 5% 80 Missing and 1 partial ⚠️
state-chain/amm/src/lib.rs 22% 61 Missing ⚠️
state-chain/amm/src/common.rs 48% 53 Missing ⚠️
state-chain/pallets/cf-pools/src/weights.rs 4% 48 Missing ⚠️
engine/src/dot/http_rpc.rs 0% 33 Missing ⚠️
state-chain/amm/src/limit_orders/v1.rs 0% 30 Missing ⚠️
... and 60 more
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #4331     +/-   ##
=======================================
- Coverage     72%     71%     -0%     
=======================================
  Files        385     390      +5     
  Lines      63554   66146   +2592     
  Branches   63554   66146   +2592     
=======================================
+ Hits       45448   47068   +1620     
- Misses     15749   16710    +961     
- Partials    2357    2368     +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

engine/src/dot/http_rpc.rs Outdated Show resolved Hide resolved
engine/src/dot/rpc.rs Outdated Show resolved Hide resolved
engine/src/dot/http_rpc.rs Outdated Show resolved Hide resolved
engine/src/dot/http_rpc.rs Outdated Show resolved Hide resolved
engine/src/dot/http_rpc.rs Outdated Show resolved Hide resolved
engine/src/dot/http_rpc.rs Show resolved Hide resolved
tracing::info!(
"Witnessing transaction_succeeded. signature: {signature:?}"
);
process_call(
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 used to be properly indented.

@kylezs kylezs merged commit 4736a3f into main Dec 11, 2023
41 checks passed
@kylezs kylezs deleted the fix/polkadot-runtime-update-not-parse branch December 11, 2023 07:36
syan095 added a commit that referenced this pull request Dec 12, 2023
…-timeout

* origin/main:
  test: latest_then adapter testing (#4322)
  Fix: CFE Witnessing, use parent block metadata when decoding events (#4331)
  chore: remove unused Tailscale access (#4205)
  chore: clear CDN cache after releasing new apt packages 🪄 (#4325)
  hotfix: correct binary-subdir for production (#4328)
  fix: build sisyphos with production profile (#4327)
  Updated banana mode to adhere to new LP API (#4326)
  `cf_pool_orders` RPC call can return all Pool Orders (#4315)
  BTC reorg script for bouncer (#4302)
  fix: broker endpoint as env var (#4317)
kylezs pushed a commit that referenced this pull request Dec 12, 2023
…4331)

* fix: use parent block when getting polkadot metadata

* chore: addressing PR comments
tomjohnburton pushed a commit that referenced this pull request Dec 12, 2023
Co-authored-by: kylezs <zsembery.kyle@gmail.com>
Co-authored-by: Martin Rieke <martin@chainflip.io>
Co-authored-by: Jamie Ford <jamie@chainflip.io>
Co-authored-by: Maxim Shishmarev <msgmaxim@gmail.com>
Co-authored-by: Alastair Holmes <42404303+AlastairHolmes@users.noreply.github.com>
Co-authored-by: Martin Rieke <121793148+martin-chainflip@users.noreply.github.com>
Co-authored-by: dandanlen <3168260+dandanlen@users.noreply.github.com>
Fix: CFE Witnessing, use parent block metadata when decoding events (#4331)
fix: sweeping before withdrawal (#4337)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants