-
Notifications
You must be signed in to change notification settings - Fork 704
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
Drop Pending Stakers 2 - Replace txs.ScheduledStaker with txs.Staker #2305
Conversation
@@ -545,3 +532,34 @@ func (e *StandardTxExecutor) BaseTx(tx *txs.BaseTx) error { | |||
avax.Produce(e.State, e.Tx.ID(), tx.Outs) | |||
return nil | |||
} | |||
|
|||
// addStakerFromStakerTx creates the staker and adds it to state. | |||
func (e *StandardTxExecutor) addStakerFromStakerTx(stakerTx txs.Staker) error { |
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 just a scaffolding. In next PRs we'll handle here (and here alone) the creation of PostDurango stakers
func NewCurrentStaker( | ||
txID ids.ID, | ||
staker txs.PostDurangoStaker, | ||
startTime time.Time, |
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 the main point of this PR. Upon drop of pending stakers, we'll explicitly pass chainTime here to create the staker object. In this PR we anticipate the structural change by spreading use of txs.Staker instead of txs.ScheduledStaker wherever possible
vms/platformvm/state/state.go
Outdated
staker, err := NewCurrentStaker(txID, stakerTx, metadata.PotentialReward) | ||
staker, err := NewCurrentStaker( | ||
txID, | ||
stakerTx.(txs.PostDurangoStaker), |
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 safe because all of our StakerTxs are also PostDurangoStakerTxs and viceversa. We type assert it for each transaction.
Still ain't pretty
vms/platformvm/state/state.go
Outdated
stakerTx, ok := tx.Unsigned.(txs.Staker) | ||
if !ok { | ||
return fmt.Errorf("expected tx type txs.Staker but got %T", tx.Unsigned) | ||
} |
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.
just moved up a fewl line. This should never happen, but always better to fail fast.
5cf9fb6
to
0deb60a
Compare
vms/platformvm/state/state.go
Outdated
stakerTx, ok := tx.Unsigned.(txs.Staker) | ||
if !ok { | ||
return fmt.Errorf("expected tx type txs.Staker but got %T", tx.Unsigned) | ||
} |
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.
just moved up a bit. Should never happen but always better to fail fast
0deb60a
to
8e8ab33
Compare
8e8ab33
to
b213740
Compare
vms/platformvm/state/state.go
Outdated
staker, err := NewCurrentStaker( | ||
txID, | ||
stakerTx, | ||
stakerTx.(txs.ScheduledStaker).StartTime(), |
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.
Can we properly check for the type cast here (and below)? I know this should never be able to fail... and that it will be removed in a followup PR once the startTime is persisted... But I'd prefer to keep the merged code "safe"
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
Why this should be merged
We chance NewCurrentStaker signature to allow specifying a staking start time that is not necessarily that indicated into the staker transaction.
How this works
Slicing #2175 up
How this was tested
CI