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

feat: Solana CCM fallback #5316

Merged
merged 13 commits into from
Oct 18, 2024
Merged

feat: Solana CCM fallback #5316

merged 13 commits into from
Oct 18, 2024

Conversation

syan095
Copy link
Contributor

@syan095 syan095 commented Oct 7, 2024 •
edited
Loading

Pull Request

Closes: PRO-1621

Checklist

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

  • I am confident that the code works.
  • I have written sufficient tests.
  • I have written and tested required migrations.
  • I have updated documentation where appropriate.

Summary

We can now witness when Solana transactions fail to execute.
The witnessed result is now part of the SolanaElections. On Program execution error, fallback transfer is created for CCMs.
Solana Ccms now require a "fallback" address as part of the cf_params.

Added integration test for the fallback mechanism.
Added storage migration:

  • Clear all Solana egress-success votes.
  • Migrated SolanaBroadcaster pallet's storage to the newer version of Solana ApiCall.

@syan095
We can now witness when Solana CCM transaction fails to execute.
419c451 
The witnessed result is now part of the SolanaElections.
On Program execution error, fallback transfer is created.

WIP: Add tests and storage migration for SolanaBroadcast and Elections
@syan095 syan095 requested a review from albert-llimos October 7, 2024 10:53
syan095 added 3 commits October 9, 2024 18:11
@syan095
Added integration test for CCM fallback mechanism.
f5a4bc1
@syan095
Merge remote-tracking branch 'origin/main' into feat/refund-failed-so…
3e48898 
…lana-ccm

* origin/main:
  chore: update polkadot sdk 1.15.2, rust to 2024-07-31 (#5306)
  chore: add sol min_prio_fee to ingress and egress estimation (#5323)
  docs: fix log filtering in readme (#5319)
  feat: cleanup aborted broadcasts (#5301)
  fix: increase init timeout for backup test (#5317)
  feat: minimum chunk size setting (#5314)
  chore: add egress fee for token account rent (#5315)
  fix: ensure unused sol deposit channels are closed (#5312)
@syan095
Added storage migration for sol ccm success witnessing.
0975259 
Clear all egress-success election votes
Migrated Broadcaster storage to newer version of ApiCall
@syan095 syan095 marked this pull request as ready for review October 10, 2024 04:31
@syan095 syan095 requested review from dandanlen and removed request for dandanlen October 10, 2024 04:35
@codecov Codecov
Copy link

codecov bot commented Oct 10, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 58.02469% with 102 lines in your changes missing coverage. Please review.

Project coverage is 71%. Comparing base (33eb5eb) to head (34f9380).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...me/src/migrations/solana_egress_success_witness.rs 0% 74 Missing ⚠️
engine/src/witness/sol/egress_witnessing.rs 0% 7 Missing ⚠️
engine/src/witness/sol.rs 0% 6 Missing ⚠️
state-chain/pallets/cf-ingress-egress/src/lib.rs 75% 5 Missing ⚠️
...te-chain/runtime/src/chainflip/solana_elections.rs 55% 4 Missing and 1 partial ⚠️
state-chain/chains/src/sol/api.rs 73% 3 Missing ⚠️
state-chain/cf-integration-tests/src/solana.rs 98% 0 Missing and 2 partials ⚠️
Additional details and impacted files 
@@          Coverage Diff           @@
##            main   #5316    +/-   ##
======================================
  Coverage     71%     71%            
======================================
  Files        490     492     +2     
  Lines      85209   85416   +207     
  Branches   85209   85416   +207     
======================================
+ Hits       60659   60896   +237     
+ Misses     21839   21790    -49     
- Partials    2711    2730    +19

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

@syan095
Call SolanaBroadcaster::egress_success() regardless of if call execution
f56c223 
was successful. This ensures call data are cleared in the broadcaster pallet.
@syan095 syan095 requested a review from dandanlen October 14, 2024 09:32
@syan095 syan095 enabled auto-merge October 14, 2024 23:04
@syan095
Merge remote-tracking branch 'origin/main' into feat/refund-failed-so…
da65a84 
…lana-ccm

* origin/main:
  ci: upgrade action version to supress deprecation warnings ⚙️ (#5330)
  feat: handle rotation tx construction failures (#5307)
  test(bouncer): add test for new utility (#5324)
  fix: keyholder check should use `HistoricalActiveEpochs` (#5325)
  feat: BTC contract swap encoding (#5311)
albert-llimos
albert-llimos reviewed Oct 15, 2024
View reviewed changes
state-chain/runtime/src/chainflip/solana_elections.rs Outdated Show resolved Hide resolved
state-chain/runtime/src/chainflip/solana_elections.rs
{
// Only Ccm calls support fallback.
if let SolanaTransactionType::CcmTransfer { fallback } = ccm_tx.call_type {
SolanaIngressEgress::do_ccm_fallback(broadcast_id, fallback);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we emit some event for the frontend to detect when a ccm transaction has reverted or we can rely on them understanding that when they see a CcmFallbackScheduled event with a broadcast Id it means that the CCM has reverted even if the egress is succesful? I presume that they use egress success as a proxy for transaction has succeeded. @acdibble

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 we have an event emitted when the fallback is triggered:
Event::<T, I>::CcmFallbackScheduled {
broadcast_id,
egress_details,
}

@syan095 syan095 added this pull request to the merge queue Oct 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
@syan095 syan095 added this pull request to the merge queue Oct 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
@syan095 syan095 added this pull request to the merge queue Oct 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
@syan095 syan095 enabled auto-merge October 17, 2024 07:06
@syan095 syan095 added this pull request to the merge queue Oct 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
@syan095 syan095 added this pull request to the merge queue Oct 17, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 17, 2024
@syan095 syan095 enabled auto-merge October 18, 2024 03:20
@syan095 syan095 added this pull request to the merge queue Oct 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 18, 2024
@msgmaxim @anton-chain @dandanlen
fix: don't exit if we can't remove a file that already doesn't exist (#…
73c973b 
…5328)

* fix: don't exit if we can't remove a file that already doesn't exist

* Update localnet/common.sh

Co-authored-by: anton-chain <anton@chainflip.io>

---------

Co-authored-by: anton-chain <anton@chainflip.io>
@dandanlen dandanlen force-pushed the feat/refund-failed-solana-ccm branch from e47b08a to 73c973b Compare October 18, 2024 11:47
@dandanlen dandanlen requested a review from a team as a code owner October 18, 2024 11:47
@dandanlen dandanlen requested review from ahasna and removed request for a team October 18, 2024 11:47
@syan095 syan095 added this pull request to the merge queue Oct 18, 2024
Merged via the queue into main with commit daa3163 Oct 18, 2024
48 of 49 checks passed
@syan095 syan095 deleted the feat/refund-failed-solana-ccm branch October 18, 2024 21:44
syan095 added a commit that referenced this pull request Oct 23, 2024
@syan095
Merge remote-tracking branch 'origin/main' into chore/unify-dependencies
8e66a27 
* origin/main:
  feat: broker can encode btc smart contract call (#5329)
  chore: localnet recreate script can use defaults (#5338)
  feat: witnessing btc smart contract swaps (#5331)
  feat: Solana CCM fallback (#5316)
  fix: scale types for pending ceremonies (#5286)
  chore: Prune historical values in Validator pallet (#5292)
  feat: expose deposit transaction hash from ingress-egress-tracker (#5320)

# Conflicts:
#	Cargo.lock
#	engine/src/witness/btc/smart_contract.rs
@syan095 syan095 mentioned this pull request Oct 24, 2024
Merged
syan095 added a commit that referenced this pull request Oct 29, 2024
@syan095
Merge remote-tracking branch 'origin/main' into feat/solana-program-s…
af8b520 
…waps-close-accounts

* origin/main: (44 commits)
  fix: expire all previous epochs (#5279)
  feat: add/update contract swaps parameters (#5343)
  chore: add address to solana logging (#5353)
  fix: ignore dust underflows in order fills rpc (#5352)
  chore: consistent naming prewitnessed (#5351)
  feat: engine-runner verifies gpg signature of old dylib when downloaded (#5339)
  feat: tainted transaction reporting (#5310)
  bug: change_utxo not always present (#5340)
  feat: structured error return types for rpcs (#5346)
  chore: unify dependencies to root cargo.toml (#5333)
  feat: Submit a slot number alongside nonce (#5297)
  chore: use node version from `.nvmrc` 📌 (#5336)
  chore: add engine account_info logging (#5347)
  chore: replace manual scale encoding for ts-scale (#5335)
  chore: more consistent params in Broker API (#5342)
  feat: broker can encode btc smart contract call (#5329)
  chore: localnet recreate script can use defaults (#5338)
  feat: witnessing btc smart contract swaps (#5331)
  feat: Solana CCM fallback (#5316)
  fix: scale types for pending ceremonies (#5286)
  ...

# Conflicts:
#	Cargo.lock
#	state-chain/chains/src/sol/api.rs
#	state-chain/pallets/cf-broadcast/src/migrations.rs
#	state-chain/pallets/cf-environment/Cargo.toml
dandanlen added a commit that referenced this pull request Oct 30, 2024
@syan095 @msgmaxim
@anton-chain @dandanlen
feat: Solana CCM fallback (#5316)
69480d6 
* We can now witness when Solana CCM transaction fails to execute.

The witnessed result is now part of the SolanaElections.
On Program execution error, fallback transfer is created.

WIP: Add tests and storage migration for SolanaBroadcast and Elections

* Added integration test for CCM fallback mechanism.

* Added storage migration for sol ccm success witnessing.
Clear all egress-success election votes
Migrated Broadcaster storage to newer version of ApiCall

* Corrected Migration in Runtime.

* Call SolanaBroadcaster::egress_success() regardless of if call execution
was successful. This ensures call data are cleared in the broadcaster pallet.

* Commented out a print statement in the unit test

* renamed a member field

* Fixed bouncer test for Solana CCMs

* Fixed bouncer lint

* fix: don't exit if we can't remove a file that already doesn't exist (#5328)

* fix: don't exit if we can't remove a file that already doesn't exist

* Update localnet/common.sh

Co-authored-by: anton-chain <anton@chainflip.io>

---------

Co-authored-by: anton-chain <anton@chainflip.io>

* fix

---------

Co-authored-by: Maxim Shishmarev <msgmaxim@gmail.com>
Co-authored-by: anton-chain <anton@chainflip.io>
Co-authored-by: Daniel <daniel@chainflip.io>
dandanlen added a commit that referenced this pull request Oct 31, 2024
@syan095 @msgmaxim
@anton-chain @dandanlen
feat: Solana CCM fallback (#5316)
c2110d7 
* We can now witness when Solana CCM transaction fails to execute.

The witnessed result is now part of the SolanaElections.
On Program execution error, fallback transfer is created.

WIP: Add tests and storage migration for SolanaBroadcast and Elections

* Added integration test for CCM fallback mechanism.

* Added storage migration for sol ccm success witnessing.
Clear all egress-success election votes
Migrated Broadcaster storage to newer version of ApiCall

* Corrected Migration in Runtime.

* Call SolanaBroadcaster::egress_success() regardless of if call execution
was successful. This ensures call data are cleared in the broadcaster pallet.

* Commented out a print statement in the unit test

* renamed a member field

* Fixed bouncer test for Solana CCMs

* Fixed bouncer lint

* fix: don't exit if we can't remove a file that already doesn't exist (#5328)

* fix: don't exit if we can't remove a file that already doesn't exist

* Update localnet/common.sh

Co-authored-by: anton-chain <anton@chainflip.io>

---------

Co-authored-by: anton-chain <anton@chainflip.io>

* fix

---------

Co-authored-by: Maxim Shishmarev <msgmaxim@gmail.com>
Co-authored-by: anton-chain <anton@chainflip.io>
Co-authored-by: Daniel <daniel@chainflip.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albert-llimos albert-llimos albert-llimos left review comments

@dandanlen dandanlen dandanlen approved these changes

@kylezs kylezs Awaiting requested review from kylezs kylezs is a code owner

@martin-chainflip martin-chainflip Awaiting requested review from martin-chainflip martin-chainflip is a code owner

@ahasna ahasna Awaiting requested review from ahasna ahasna is a code owner automatically assigned from chainflip-io/platform

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@syan095 @dandanlen @albert-llimos @msgmaxim