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-log-message-double-stake #2540
Conversation
…in the same stake call.
@@ -178,7 +179,9 @@ func (stp *stakingToPeer) UpdateProtocol(body *block.Body, nonce uint64) error { | |||
// no data under key -> peer can be deleted from trie | |||
if len(data) == 0 { | |||
err = stp.peerState.RemoveAccount(blsPubKey) | |||
log.LogIfError(err, "staking to protocol RemoveAccount blsPubKey", blsPubKey) | |||
if err != nil { | |||
log.Debug("staking to protocol RemoveAccount", "error", err, "blsPubKey", hex.EncodeToString(blsPubKey)) |
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.
Please use stp.pubkeyConv.Encode instead of hex.EncodeToString(blsPubKey)
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.
this is bls key, not bech32.
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.
pubkeyConv is of type hexPkConverter. Can be left as it is
func checkDoubleBLSKeys(blsKeys [][]byte) bool { | ||
mapKeys := make(map[string]struct{}) | ||
for _, blsKey := range blsKeys { | ||
_, found := mapKeys[string(blsKey)] |
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.
👍
@@ -178,7 +179,9 @@ func (stp *stakingToPeer) UpdateProtocol(body *block.Body, nonce uint64) error { | |||
// no data under key -> peer can be deleted from trie | |||
if len(data) == 0 { | |||
err = stp.peerState.RemoveAccount(blsPubKey) | |||
log.LogIfError(err, "staking to protocol RemoveAccount blsPubKey", blsPubKey) | |||
if err != nil { | |||
log.Debug("staking to protocol RemoveAccount", "error", err, "blsPubKey", hex.EncodeToString(blsPubKey)) |
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.
staking to peer ?
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.
all logs here are with staking to protocol.
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.
ok
@@ -776,6 +792,11 @@ func (s *stakingAuctionSC) stake(args *vmcommon.ContractCallInput) vmcommon.Retu | |||
return vmcommon.UserError | |||
} | |||
|
|||
if s.flagDoubleKey.IsSet() && checkDoubleBLSKeys(blsKeys) { | |||
s.eei.AddReturnMessage("invalid arguments, found same bls key twice") |
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.
more than ones/many times ?
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.
it stops at found twice.
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.
ok
ffb961a
vm/systemSmartContracts/auction.go
Outdated
newList := make([][]byte, 0) | ||
mapExistingKeys := make(map[string]struct{}) | ||
for _, blsKey := range registrationData.BlsPubKeys { | ||
if _, found := mapExistingKeys[string(blsKey)]; found { |
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.
multiple lines?
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.
done
return vmcommon.UserError | ||
} | ||
|
||
err := s.eei.UseGas(s.gasCost.MetaChainSystemSCsCost.Stake) |
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.
So the cost for cleaning is s.gasCost.MetaChainSystemSCsCost.Stake?
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.
yes.
vm/systemSmartContracts/auction.go
Outdated
return vmcommon.UserError | ||
} | ||
|
||
if len(registrationData.BlsPubKeys) == 0 { |
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.
Could be <= 1
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.
done
mapExistingKeys[string(blsKey)] = struct{}{} | ||
newList = append(newList, blsKey) | ||
} | ||
|
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.
You can add here:
if !changesMade {
return vmcommon.Ok
}
To avoid useless make and set with the same content, and remove L775 and L781
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.
done
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.
System Test Passed
fix log message. added protection against staking same key two times in the same stake call.