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

dry_run always returns OK() #4124

Merged
merged 4 commits into from
Oct 19, 2023
Merged

dry_run always returns OK() #4124

merged 4 commits into from
Oct 19, 2023

Conversation

marcellorigotti
Copy link
Contributor

@marcellorigotti marcellorigotti commented Oct 16, 2023

Pull Request

Related: PRO-917

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

  • Modified dry_run to always return Ok() until we fixed it
  • Deleted auction check when calling redeem from CLI -> check was preventing redemption during auction_phase even if the validator was not bidding

@@ -150,11 +150,12 @@ impl<
call: RuntimeCall,
at: Option<state_chain_runtime::Hash>,
) -> Result<Bytes> {
Ok(self
let _ = self
Copy link
Contributor

@kylezs kylezs Oct 16, 2023

Choose a reason for hiding this comment

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

We can just delete the old code

(whoever does the real fix can either look at this PR to see what was removed, or, because they'll be on main, they'll already have it)

@codecov
Copy link

codecov bot commented Oct 16, 2023

Codecov Report

Merging #4124 (c464d1e) into release/0.10 (cd90f95) will increase coverage by 0%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##           release/0.10   #4124   +/-   ##
============================================
  Coverage            72%     72%           
============================================
  Files               372     372           
  Lines             59360   59353    -7     
  Branches          59360   59353    -7     
============================================
+ Hits              42665   42676   +11     
+ Misses            14571   14558   -13     
+ Partials           2124    2119    -5     
Files Coverage Δ
api/lib/src/lib.rs 31% <0%> (+1%) ⬆️

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kylezs
Copy link
Contributor

kylezs commented Oct 16, 2023

Cheers

@@ -147,14 +146,10 @@ impl<
{
async fn dry_run(
&self,
call: RuntimeCall,
at: Option<state_chain_runtime::Hash>,
_call: RuntimeCall,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a todo or comment here (referencing the Linear issue)?

@dandanlen dandanlen merged commit 9b0f4c6 into release/0.10 Oct 19, 2023
44 checks passed
@dandanlen dandanlen deleted the fixDryRun branch October 19, 2023 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants