chainflip-io / chainflip-backend

The Chainflip backend repo, including the Chainflip Node and CFE.
50 stars 14 forks source link

chore: solana serialize broadcast data #5229

Closed albert-llimos closed 2 months ago

albert-llimos commented 2 months ago

Closes: PRO-1634

Checklist

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

Summary

Use serialized data as the transaction passed to the engine instead of the regular transaction. The regular transaction is quite annoying/difficult to serialize into the raw transaction in case it needs to be broadcasted manually

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 86.26374% with 25 lines in your changes missing coverage. Please review.

Project coverage is 70%. Comparing base (8fa1f81) to head (9a73c5b). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
state-chain/chains/src/sol/api.rs 0% 6 Missing and 2 partials :warning:
state-chain/chains/src/sol/benchmarking.rs 0% 7 Missing :warning:
...e-chain/pallets/cf-cfe-interface/src/migrations.rs 0% 4 Missing :warning:
...ntime/src/migrations/serialize_solana_broadcast.rs 83% 3 Missing and 1 partial :warning:
engine/src/state_chain_observer/sc_observer.rs 0% 1 Missing :warning:
state-chain/chains/src/sol.rs 0% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #5229 +/- ## ===================================== - Coverage 70% 70% -0% ===================================== Files 480 483 +3 Lines 86746 86778 +32 Branches 86746 86778 +32 ===================================== - Hits 61062 61017 -45 - Misses 22388 22458 +70 - Partials 3296 3303 +7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dandanlen commented 2 months ago

I'll fix clippy and queue for merge.

kylezs commented 2 months ago

This time it's not flakiness, I'm betting it's because we use this type in AwaitingBroadcasts

#[pallet::storage]
    pub type AwaitingBroadcast<T: Config<I>, I: 'static = ()> =
        StorageMap<_, Twox64Concat, BroadcastId, BroadcastData<T, I>, OptionQuery>;

inside BroadcastData so we require a migration

dandanlen commented 2 months ago

@albert-llimos you probably have more important things to do before the weekend - feel free to concentrate on other things, someone else can deal with sorting this out.

albert-llimos commented 2 months ago

@albert-llimos you probably have more important things to do before the weekend - feel free to concentrate on other things, someone else can deal with sorting this out.

Thanks, I really doubt I'll have the time for that. At least it's for 1.7 so there's no rush but it'd be great if someone could sort it out at some point :pray: