-
Notifications
You must be signed in to change notification settings - Fork 268
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 wrong casting to long #2841
Fix wrong casting to long #2841
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Manifest Files |
long validationPeriodEndBlock = proposedFederation.getCreationBlockNumber() + | ||
bridgeConstants.getFederationConstants().getValidationPeriodDurationInBlocks(); | ||
|
||
return rskExecutionBlock.getNumber() < validationPeriodEndBlock; |
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.
also replaced the <=
for a <
to follow the same logic as in migration period (i.e. not considering the ending block as part of the period)
@@ -912,7 +912,7 @@ private void recreateSvpSpendTransaction() { | |||
.setScriptSig(createBaseP2SHInputScriptThatSpendsFromRedeemScript(flyoverRedeemScript)); | |||
|
|||
// add output | |||
svpSpendTx.addOutput(Coin.valueOf(1762), federationSupport.getActiveFederationAddress()); | |||
svpSpendTx.addOutput(Coin.valueOf(2114L), federationSupport.getActiveFederationAddress()); |
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.
2114 = 1762 * 1.2
:)
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.
And where did 1762 come from?
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.
updated
* or -1 if no proposed federation exists. | ||
*/ | ||
public Long getProposedFederationCreationTime(Object[] args) { | ||
logger.trace("getProposedFederationCreationTime"); | ||
|
||
return bridgeSupport.getProposedFederationCreationTime() | ||
.map(Instant::toEpochMilli) | ||
.map(Instant::getEpochSecond) |
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.
Now I wonder if we should also apply this change to active and retiring feds
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 thought about that. I can create a task in the backlog so we dont forget
|
||
return feePerKbSupport.getFeePerKb() | ||
.multiply(svpSpendTransactionSize * backupSizePercentage) | ||
.multiply(svpSpendTransactionBackedUpSize) |
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.
.multiply(svpSpendTransactionBackedUpSize) | |
.multiply(svpSpendTransactionBackedUpSize) | |
.multiply(1200) // Add 20% just to be sure the amount sent will be enough | |
.divide(1000) |
Just an idea, maybe we encapsulate the increase here?
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 found the other way clearer. We would end up having two .divide(1000)
in a row, kind of confusing. Wdyt?
@@ -567,7 +567,7 @@ void processSvpSpendTransaction_createsExpectedTransactionAndSavesTheValuesAndLo | |||
assertSvpFundTxSignedWasRemovedFromStorage(); | |||
|
|||
assertLogPegoutTransactionCreated(logs, svpSpendTransactionUnsigned); | |||
Coin valueSentToActiveFed = Coin.valueOf(1762); | |||
Coin valueSentToActiveFed = Coin.valueOf(2114L); |
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.
Where is this magic number coming from?
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.
updated
@@ -912,7 +912,7 @@ private void recreateSvpSpendTransaction() { | |||
.setScriptSig(createBaseP2SHInputScriptThatSpendsFromRedeemScript(flyoverRedeemScript)); | |||
|
|||
// add output | |||
svpSpendTx.addOutput(Coin.valueOf(1762), federationSupport.getActiveFederationAddress()); | |||
svpSpendTx.addOutput(Coin.valueOf(2114L), federationSupport.getActiveFederationAddress()); |
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.
And where did 1762 come from?
Quality Gate passedIssues Measures |
c235fe0
into
feature/powpeg_validation_protocol-phase3
svpIsOngoing
readability