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.

Sign up for GitHub

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

Jump to bottom

difftest core: Amend diff model with throttle changes (normal operation) #533

Closed
2 tasks done
danwt opened this issue Nov 30, 2022 · 3 comments
Closed
2 tasks done

difftest core: Amend diff model with throttle changes (normal operation) #533

danwt opened this issue Nov 30, 2022 · 3 comments
Assignees
@danwt
Labels
scope: MBT model based testing scope: testing Code review, testing, making sure the code is following the specification.

Comments

@danwt
Copy link
Contributor

danwt commented Nov 30, 2022 •
edited
Loading

Problem

The slash throttle PR changes the way things work under normal operation (and abnormal operation). The diff test model should be changed to account for this.

Closing criteria

Update the diff test core model so that the tests pass using the throttle when the Replenish Period and Allowance params are large.

Problem details

The throttle should work almost as normal when the slash allowance is very large.

TODOs

  • Labels have been added for issue
  • Issue has been added to the ICS project
@danwt danwt added scope: testing Code review, testing, making sure the code is following the specification. ccv-core scope: MBT model based testing labels Nov 30, 2022
@danwt danwt moved this to Todo in Replicated Security Nov 30, 2022
@danwt danwt self-assigned this Nov 30, 2022
@danwt danwt changed the title difftest core: Amend diff model with throttle changes difftest core: Amend diff model with throttle changes (normal operation) Nov 30, 2022
This was referenced Nov 30, 2022
Closed
Merged
@danwt
Copy link
Contributor Author

danwt commented Nov 30, 2022

cc @smarshall-spitzbart

@shaspitz
Copy link
Contributor

Looks good, this issue is about using diff tests as a regression test, and will need to be fixed before #462 is merged. Whereas #534 is future work to test the throttling logic (not regression testing)

@jtremback jtremback moved this from Todo to Next in Replicated Security Dec 5, 2022
@jtremback jtremback moved this from Next to Done in Replicated Security Dec 8, 2022
@jtremback jtremback moved this from Done to Waiting for review in Replicated Security Dec 8, 2022
@danwt danwt mentioned this issue Dec 8, 2022
Closed
@danwt
Copy link
Contributor Author

danwt commented Dec 8, 2022

See #571

This was referenced Dec 9, 2022
Closed
Merged
danwt added a commit that referenced this issue Dec 13, 2022
@danwt
#533 Adds normal operation diff testing
fcf29bc 
* reb

* reb

* Del unused

Co-authored-by: Daniel <[email protected]>
@danwt danwt closed this as completed Dec 13, 2022
Repository owner moved this from Waiting for review to Done in Replicated Security Dec 13, 2022
jtremback added a commit that referenced this issue Dec 16, 2022
@shaspitz @jtremback
@mpoke @sainoe @MSalopek @danwt
Slash packet throttling (#462)
ae66785 
* wip

* Update module.go

* wip

* tests pass

* Update relay.go

* Update relay.go

* Update relay.go

* Update relay.go

* wip

* still wip

* panic with too many packets

* Update relay.go

* wip

* checkpoint

* Update throttle.go

* make relay.go closer to main

* merge fixes

* Update expected_keepers.go

* Update throttle_test.go

* Update throttle.go

* second queue is now implemented

* smalls

* Update throttle.go

* removed pointer silliness

* Update throttle_test.go

* test improvements

* small

* where it's called

* Update relay.go

* comments n stuff

* Update throttle.go

* Update throttle.go

* comments

* wip

* callbacks

* cleans

* mas tests

* Update README.md

* less diff

* mas

* keys

* wip

* changes

* Update params.go

* size constraints

* Update keys_test.go

* Update throttle_test.go

* wip

* on recv new behavior and test

* on recv slash packet

* so close

* clean

* Update slashing.go

* cleans

* wip

* params

* wip

* tests

* changes

* mas

* Update README.md

* Update README.md

* Update README.md

* sorry for the friday night emails

* Update instance_test.go

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* works

* Update generic_setup.go

* Update setup.go

* smol

* small

* path to ccv chan setup

* todos

* Update setup.go

* Create debug_test.go

* democ

* Update debug_test.go

* setup all ccv channels

* bump to main

* another bump, missed one

* fix after merge main

* fixes

* Update slashing.go

* expired client tests

* checks

* fixed the stuff

* smol

* changes

* updates

* cleans

* clean

* todo

* fixes

* cleans

* Update slashing.go

* Update slashing.go

* mod tidy

* fix build errs

* test fixes

* Update slashing.go

* Update throttle.go

* base rounding

* mas unit tests

* updates

* comments

* comments

* cleans

* clean

* small

* sin gas

* more understandable logic

* crypto rand

* utils

* one e2e

* helpers

* Update slashing.go

* helpers

* cleans

* shiz works

* tcs

* smalls

* un mas

* allowance changing test

* smol

* smol

* e2e tests are done

* lets try this

* Key Assignment -> goc-december (#527)

* add MsgAssignConsumerKey

* add MsgAssignConsumerKey

* fix package name

* add keys

* add keeper methods for key assignment

* handle MsgAssignConsumerKey

* map addresses in slash requests

* prune old consumer addresses

* move AssignConsumerKey logic to keeper

* update consumer initial valset

* add ApplyKeyAssignmentToValUpdates

* fix client creation

* do not check init valset on consumer

* clean state on val removal

* fix TestAssignConsensusKeyForConsumerChain

* delete on val removal

* remove reverse mapping on val removal

* remove pending key assignment in EndBlock

* add query endpoints
add summary of indexes
change ConsumerValidatorByVscID to ConsumerAddrsToPrune

* Refactor AssignConsumerKey for clarity (IMO)

* finish key assignment genesis code- untested

* FIxed mocks compile issue - not sure if it works right though.

* add test for init and export genesis

* set after get in AssignConsumerKey

* enable AssignConsumerKey to be called twice

* remove key assignment on chain removal

* apply some review comments

* fix bug: two validator with same consumer key

* rename key: ConsumerValidatorsByVscIDBytePrefix -> ConsumerAddrsToPruneBytePrefix

* PendingKeyAssignment -> KeyAssignmentReplacements

* msg.ProviderAddr is a validator addr

* fix: key assignment genesis tests (#517)

* Fix consumer init genesis test

* fix provider genesis tests

* fix key assignement handler

* fix linter

* fix merge conflict

* fix ProviderValidatorAddress

* remove unused expectation

Co-authored-by: Marius Poke <[email protected]>

* add key assignment CRUD operations unit tests (#516)

* test val consumer key related CRUD

* test val consumer addr related CRUD

* test pending key assignments related CRUD

* refactor after review session

* refactor after review session

* add prune key CRUD tests

* renamings in testfiles

* improve KeyAssignmentReplacement set and get

* remove ApplyKeyAssignmentToInitialValset (redundant)

* add invariant to docstring of AppendConsumerAddrsToPrune

* fix address conversion

* adding e2e tests

* fix linter

* add queries; setup integration tests (#519)

* add queries; setup integration testse

* test key assignment before chain start

* fix state queries; refactor

* rm extra comment

* rm unused action field

* bump voting times in all tests

* add provider address query to tests

* Adds some very basic random testing and unit tests (#522)

* Adds imports

* Does multi iterations: fails!

* Handle errs

* checkpoint debug

* Pre introduce dynamic mock

* Issue seems to be resolved

* Removes prints in key asisgn

* Removes debug, pre reintroduce all test features

* Fix some magic numbers, bring back prune check

* Pre rework initial assignments

* Refactor and tidyup

* Better docs, clarity, org

Co-authored-by: Daniel <[email protected]>

* Enable key assignment testing for all e2e tests (#524)

* split CCVTestSuite.setupCallback in two

* pre-assign keys for all vals of first consumer

* fix linter

* remove TestConsumerGenesis

Co-authored-by: mpoke <[email protected]>
Co-authored-by: Simon Noetzlin <[email protected]>
Co-authored-by: MSalopek <[email protected]>
Co-authored-by: Daniel T <[email protected]>
Co-authored-by: Daniel <[email protected]>

* Fix errors in merge commit, comment out failing TestRelayAndApplySlashPacket test

* add simon's test fixes

* Update state.go

* last replenish time -> last full time

* readme

* increment i

* shared method

* iteration change

* Meter allowance lockstep (#553)

changes

* log

* requested key format changes (#560)

changes

* Throttle garbage collection (#557)

* changes

* Update proposal.go

* add log

* Update throttle_test.go

* fixes

* update invariant

* Throttle bug fixes + req refactors (#565)

* fixes

* Update throttle.go

* fix tests

* set replenish frac = 1.0 for all test runs

* rm unmarshal func

* req refactors

* small lint fix

* comment adjustment

* found <-> jailed order swap

* 100% change

* weird

* merge fixes to build

* tests now pass

* name change

* better ordering tests

* remove integration test diff

* avoid double call to address mapping

* Update throttle.go

* 0 included in iteration start

* naming refactors

* Update keys_test.go

* more refactors

* clarify allowance terminology

* update doc with explanation on min value

* md clarification

* Update throttle.md

* swap replenish order

* add max limit note

* #533 Adds normal operation diff testing

* reb

* reb

* Del unused

Co-authored-by: Daniel <[email protected]>

* progress save

* cleans

* name change

* Bugfix (#605)

* Circuit breaker refactor (#606)

* quick fix

* small key correction

* smol

* don't store time length

* use big endian, shawn you dingus

Co-authored-by: Jehan <[email protected]>
Co-authored-by: mpoke <[email protected]>
Co-authored-by: Simon Noetzlin <[email protected]>
Co-authored-by: MSalopek <[email protected]>
Co-authored-by: Daniel T <[email protected]>
Co-authored-by: Daniel <[email protected]>
Co-authored-by: Jehan Tremback <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@danwt danwt

Labels
scope: MBT model based testing scope: testing Code review, testing, making sure the code is following the specification.
Projects
No open projects
Replicated Security
Status: Done
Milestone
No milestone
Development

No branches or pull requests
2 participants
@danwt @shaspitz