Skip to content

Conversation

@azmenak
Copy link

@azmenak azmenak commented Jan 7, 2023

This is just for reference.

Basic and untested addition to the proxy vault which could be used to support staking for Custody

@azmenak azmenak requested a review from a team as a code owner January 7, 2023 00:54
pub var lastNonce: Int64
access(self) let flowVault: @FungibleToken.Vault
access(self) let publicKey: [UInt8]
access(self) let nodeDelegator: @FlowIDTableStaking.NodeDelegator
Copy link
Member

Choose a reason for hiding this comment

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

Is this contract already deployed on a network? You aren't able to add fields in upgrades, so you'd need to find another way to do this if it is already deployed

access(self) let nodeDelegator: @FlowIDTableStaking.NodeDelegator

init(publicKey: [UInt8]) {
init(publicKey: [UInt8], stakingNodeID: String) {
Copy link
Member

Choose a reason for hiding this comment

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

It is right to register a delegator for the user by default, or should there be a different method to allow them the option to register only if they want to?

Comment on lines 161 to 172
var data: [UInt8] = "delegate".toBytes()
data = data.concat(amount.toBigEndianBytes())
data = data.concat(nonce.toBigEndianBytes())

// Verify the signature.
let key = PublicKey(
publicKey: self.publicKey,
signatureAlgorithm: SignatureAlgorithm.ECDSA_secp256k1
)
if !key.verify(signature: sig, signedData: data, domainSeparationTag: "FLOW-V0.0-user", hashAlgorithm: HashAlgorithm.SHA3_256) {
panic("Invalid meta transaction signature")
}
Copy link
Member

Choose a reason for hiding this comment

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

A lot of this code is boilerplate and is the same between the different methods. Would it be possible to split some of it out into utility methods to avoid boilerplate?

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.

2 participants