-
Notifications
You must be signed in to change notification settings - Fork 106
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
go/oasis-test-runner: governance workload #3603
Conversation
35f5216
to
3217fd4
Compare
Codecov Report
@@ Coverage Diff @@
## master #3603 +/- ##
==========================================
+ Coverage 67.19% 67.23% +0.03%
==========================================
Files 390 390
Lines 36507 36540 +33
==========================================
+ Hits 24532 24566 +34
+ Misses 8516 8512 -4
- Partials 3459 3462 +3
Continue to review full report at Codecov.
|
a049a24
to
8aaf720
Compare
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 a very quick look, still need to check the actual governance workload 😁
8aaf720
to
fd62866
Compare
epochtime "github.com/oasisprotocol/oasis-core/go/epochtime/api" | ||
governance "github.com/oasisprotocol/oasis-core/go/governance/api" | ||
staking "github.com/oasisprotocol/oasis-core/go/staking/api" | ||
"github.com/oasisprotocol/oasis-core/go/upgrade/api" |
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.
"github.com/oasisprotocol/oasis-core/go/upgrade/api" | |
upgrade "github.com/oasisprotocol/oasis-core/go/upgrade/api" |
// Load the validator entity nonce and cast vote. | ||
// TODO: this would potentially need to be retried with new nonces if | ||
// ever more workloads use the validator entity. |
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 comment can be removed as g.FundSignAndSubmitTx
does exactly that.
if err != nil { | ||
return fmt.Errorf("querying block: %w", err) | ||
} | ||
epoch2, err = cnsc.GetEpoch(g.ctx, block.Height+10) |
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 will not work with the new beacon as GetEpoch
there performs a query which will fail if you give it a future height. Also with the new beacon there is no way to know for sure when the next transition will happen.
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.
Hm yeah that's right. Ok i'll update the workload and turn things around a bit:
- first a request will be made (e.g. submit proposal, vote ...)
- after the request the workload will check if the request should have failed or succeed based on the state at that height
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.
actually simplified it a bit more even. In case the unexpected result happens (e.g. a proposal is successfully submitted when it shouldn't be - or otherwise) the workload checks if the epoch transition just happened, and in case it did it doesn't fail the whole workload and just emits an 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.
Yeah that sounds like the right solution, thanks!
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.
Np! thanks for noticing that this wouldn't work with the new beacon 😁 forgot about it.
// XXX: this makes sure that any pending upgrades that are about to be executed are | ||
// canceled. When txsource suite supports handling upgrades mid-run, remove this part. |
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.
Is there an issue for tracking this?
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.
will open one
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.
opened #3609
fd62866
to
2e9c02e
Compare
@@ -22,6 +22,7 @@ import ( | |||
"github.com/oasisprotocol/oasis-core/go/common/quantity" | |||
consensus "github.com/oasisprotocol/oasis-core/go/consensus/api" | |||
"github.com/oasisprotocol/oasis-core/go/consensus/api/transaction" | |||
"github.com/oasisprotocol/oasis-core/go/governance/api" |
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.
"github.com/oasisprotocol/oasis-core/go/governance/api" | |
governance "github.com/oasisprotocol/oasis-core/go/governance/api" |
2e9c02e
to
b813198
Compare
b813198
to
f3af4e7
Compare
UpgradeCancelMinEpochDiff
before execution, to ensure no upgrades actually happen during the txsource test. In future we could/should probably also support this in the txsource test.TODO: