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

feat(oracle-feeder): initialise core logic #1010

Merged
merged 11 commits into from
Oct 20, 2022

Conversation

testinginprod
Copy link
Collaborator

@testinginprod testinginprod commented Oct 18, 2022

Description

Adds the base layer of logic for the feeder sidecar process. The PR programs the feeder type against interfaces which will be implemented in follow-up PRs.

Purpose

Helps fulfill the decentralized oracle.

@testinginprod testinginprod requested a review from a team as a code owner October 18, 2022 15:12
})
})

t.Run("events stream invalid val set - has no in validators", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
t.Run("events stream invalid val set - has no in validators", func(t *testing.T) {
t.Run("events stream invalid val set - has out validators", func(t *testing.T) {

feeder/types/types.go Outdated Show resolved Hide resolved
feeder/types/types.go Show resolved Hide resolved
feeder/feeder.go Outdated

// init val set
select {
case initValidators := <-stream.ValidatorSetChanged():
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this work if the validator joins late, meaning after the chain has already started?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there's a listener which listens to the current validator set changes, although we don't make use of this because of: https://github.com/NibiruChain/nibiru/blob/master/x/oracle/keeper/update_exchange_rates.go#L30

So the actual check is: "Validator.Bonded()", but can an unbonding validator be on the Active set?

Other implementations there's one in umee and one in terra, from what I've seen, did not listen to val set changes or to any change to validators at all.

I think I can refine this logic, but in a separate PR as it might complicate things a lot based on what do we need to listen for

feeder/feeder.go Outdated

func (f *Feeder) startNewVotingPeriod(vp types.VotingPeriod) {
/*
TODO(testinginprod): we need to refine this logic, other implementers did not handle this case as far as i can see.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's not harmful for non-validators to send the tx, since on-chain we only grab the votes by the current validator set. I believe we can leverage the collections API to iterate through the current validator set and do point reads, so a flood of txs from random non-validator addresses won't slow down the iteration logic.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would just cost the non-validators sending votes gas, which is fine. The gas they pay gets accrued to the actual validators.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah Ithis was my plan, who runs this assumes they're a validator in the active set, worst case we could add a check which blocks txs from non validators which are bonded so all gets blocked at checkTX and feeder runners which are not part of the bonded set don't lose money.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually,

if val := k.StakingKeeper.Validator(ctx, validatorAddr); val == nil || !val.IsBonded() {

the check is already in place, so we can assume the tx from non validators (or validators which just unbonded) will be blocked at CheckTX level and not cause any loss of funds in failed txs.

Perhaps it might even make sense to just simplify the code further and get rid of the val set changes check.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you can check if the tx is coming from an address that's part of the validator set because that's a stateful check and CheckTx is supposed to be stateless. Either the tx will fail in DeliverTx after it's included in the block, or we can add an AnteHandler to fail early and reduce the gas paid by the non-validator, though one could argue that taking away gas fees is important for preventing DoS attacks from this tx.

tf.validatorSetChanges <- expected
time.Sleep(10 * time.Millisecond)

require.Len(t, tf.f.validatorSet, len(expected.In)+1)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not necessary len(expected.In)+1, but rather the initial len(tf.validatorSet)+1.

feeder/feeder_test.go Show resolved Hide resolved
feeder/feeder.go Outdated Show resolved Hide resolved
feeder/feeder.go Outdated Show resolved Hide resolved
@testinginprod testinginprod enabled auto-merge (squash) October 20, 2022 09:59
@testinginprod testinginprod merged commit a65c41b into master Oct 20, 2022
@testinginprod testinginprod deleted the mercilex/oracle-feeder-init branch October 20, 2022 11:45
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.

None yet

2 participants