z:acceptance governance fails/flakes: No quorum #10708

dckc · 2024-12-16T23:01:02Z

I'm getting a ci failure in an unrelated PR:
#10692 (comment)

This readLatestHead immediate assumes vstorage updates by the time the deadline is reached; I don't think that's necessarily the case:

agoric-sdk/a3p-integration/proposals/z:acceptance/governance.test.js

Lines 100 to 106 in 3e27c74

    
           await waitUntil(deadline); 
        
           const latestOutcome = await readLatestHead( 
        
             'published.committees.Economic_Committee.latestOutcome', 
        
           ); 
        
           t.log('Verifying latest outcome', latestOutcome); 
        
           t.like(latestOutcome, { outcome: 'win' });

A retryUntilCondition seems in order.

I don't know whether it's a flake. It failed again.

https://github.com/Agoric/agoric-sdk/actions/runs/12345315961/job/34449230334?pr=10692#step:10:8388

cc @frazarshad @LuqiPan @Chris-Hibbert

The text was updated successfully, but these errors were encountered:

dckc · 2024-12-16T23:12:26Z

On second thought, I wonder if retryUntilCondition is really going to help: we get no quorum, not an empty read.

But the no quorum could be the outcome of an earlier question.

Be sure to check that latestQuestion has updated to the relevant question before checking latestOutcome.

dckc · 2024-12-16T23:14:56Z

possibly out of scope, but while you're in there... I'd like the previousOfferId arg of governanceDriver.proposeVaultDirectorParamChange become optional; the function should do the .find in the wallet purses and such.

anilhelvaci · 2024-12-17T08:47:36Z

Investigation

Param Change Tx

2024-12-16T04:10:17.755Z SwingSet: vat: v43: walletFactory.fromBridge: { blockHeight: 1765, blockTime: 1734322212, owner: 'agoric1c9gyu460lu70rtcdp95vummd6032psmpdx7wdy', 
spendAction: '{
"body":"#{\\"method\\":\\"executeOffer\\",\\"offer\\":{\\"id\\":\\"propose-1734322211878\\",\\"invitationSpec\\":{\\"invitationMakerName\\":\\"VoteOnParamChange\\",\\"previousOffer\\":\\"charter-1734320369849\\",\\"source\\":\\"continuing\\"},
\\"offerArgs\\":{\\"deadline\\":\\"+1734322222\\",\\"instance\\":\\"$0.Alleged: BoardRemoteInstanceHandle\\",\\"params\\":{\\"ChargingPeriod\\":\\"+400\\"},\\"path\\":{\\"paramPath\\":{\\"key\\":\\"governedParams\\"}}
},\\"proposal\\":{}}}","slots":["board00360"]}', type: 'WALLET_SPEND_ACTION' }

Voting Duration

agoric-sdk/a3p-integration/proposals/z:acceptance/test-lib/governance.js

Line 147 in 9e124c3

generateVaultDirectorParamChange(offerId, 10, params, path),

Thoughts

The deadline's GMT conversion is Mon Dec 16 2024 04:10:22 GMT+0000. Given that the voting durations is 10 seconds, the transaction is sent at prepared and sent at Mon Dec 16 2024 04:10:11 GMT+0000 (double checked with the offerId propose-1734322211878). However, when we look at the block timestamp of the governance transaction: 2024-12-16T04:10:17.755Z, it is clear that there's a delay around 5 seconds.

We can verify the delay by looking at when the No Quorum is printed:

2024-12-16T04:10:28.658Z SwingSet: xsnap: v43: UnhandledPromiseRejectionWarning: No quorum
2024-12-16T04:10:28.661Z SwingSet: xsnap: v82: UnhandledPromiseRejectionWarning: No quorum
2024-12-16T04:10:28.663Z SwingSet: xsnap: v9: UnhandledPromiseRejectionWarning: No quorum

The deadline should be at around 2024-12-16T04:10:22 however it is actually 2024-12-16T04:10:28. So the 5 secondish delay is now ceils up to 6 seconds.

And we can also see that the Econ Committee members' votes are getting into block (2024-12-16T04:10:32) AFTER the quorum is reached:

2024-12-16T04:10:32.772Z SwingSet: vat: v43: walletFactory.fromBridge: { blockHeight: 1768, blockTime: 1734322227, owner: 'agoric1wrfh296eu2z34p6pah7q04jjuyj3mxu9v98277', 
spendAction: '{"body":"#{\\"method\\":\\"executeOffer\\",\\"offer\\":{\\"id\\":\\"propose-1734322229372\\",
\\"invitationSpec\\":{\\"invitationArgs\\":[[{\\"changes\\":{\\"ChargingPeriod\\":\\"+400\\"}}],\\"$0.Alleged: BoardRemoteQuestionHandle\\"],\\"invitationMakerName\\":\\"makeVoteInvitation\\",
\\"previousOffer\\":\\"gov-committee-1734320362167\\",\\"source\\":\\"continuing\\"},\\"proposal\\":{}}}","slots":["board061141"]}', type: 'WALLET_SPEND_ACTION' }

This whole situation, I believe, is due to a conjunction with the auction's clockStep. When there is a clockStep, the transactions are delayed to later blocks to be processed. Since the votingDuration is only 10 seconds and these delays take around 5-6 seconds. we end up with an election flow that is out of sync.

Solution

Increase the votingDuration. Start with 20 seconds.

Jorge-Lopes · 2024-12-17T09:12:40Z

The issues here described are addressed in the PR #10555

refs: #10708

…10555) closes: https://github.com/Agoric/BytePitchPartnerEng/issues/24 closes: #10708 ## Description This PR has the objective of extending the a3p-integration acceptance proposal test coverage. More specifically, it intend to test that: - Configured EC governed parameters are intact following the contract upgrade - Governance proposals history is visible The test plan to verify that the parameters governed by the economicCommittee are intact when the contract is upgraded is: - Propose a parameter change - Verify that the value was updated - Execute a null upgrade to the respective contract - Verify that the value kept the same Although more contracts are governed by economicCommittee, the ones selected to be included in this test case were `vaultFactory` and `provisionPool`. A detailed discussion on the rationale behind this decision can be found at https://github.com/Agoric/BytePitchPartnerEng/issues/24 Note: The existing governance test and helper functions were refactored to allow the `makeGovernanceDriver` function to be contract agnostic, in order to be reused by vaultFactory, provisionPool and any other contract that is intend to be tested in the future. The test plan is to verify that the vstorage node published.committees.Economic_Committee.latestQuestion displays the history of proposed Economic Committee parameter changes is: - Use the readHistory method to fetch all parameter change proposals from latestQuestion node, starting from block height 0. - Verify that returned value is not an empty list - Construct a list of expected parameters changes proposed by the economic committee prior to the governance test. - Compare the parameters keys of both lists match. ### Security Considerations No security considerations. ### Scaling Considerations The "Governance proposals history" test compares vstorage values against a hardcoded list, requiring manual updates to expectedParametersChanges for new proposals or tests that executes a EC governed param changes made prior to this test. To prevent the halting the test when the developer does not update the hardcoded list, this test is only asserting that the value returned from the published.committees.Economic_Committee.latestQuestion vstorage node is not empty. Then it will compare both lists and if any mismatches is found, it will: - print an error message notifying the mismatch. - print both current and expected changes proposals records Note that this solution may risks oversight and could lead to latent issues, so I am open a implement a new design if desired. ### Documentation Considerations We propose adding a table to the z:acceptance README file. This table will: - List the parameters being modified during acceptance test execution. - Identify where and why these changes occur, providing developers with a clear understanding of parameter dependencies. An [issue](https://github.com/Agoric/BytePitchPartnerEng/issues/43) was created to address this task. ### Testing Considerations The methods exported by `makeVstorageKit` at the `@agoric/client-utils` package, seems to display an unexpected behaviour, more specifically, when using the `readFully` method to query Vstorage node full history. As a workaround, I updated the `makeVStorage` function at `z:acceptance/test-lib/rpc.js` to follow the same implementation as the `multichain-testing/tools/batchQuery.js.` The rational behind this decision is explained in detail on the issue #10574 If desired, I can add these changes to `@client-utils/src/vstorage.js` in a future PR, contribute to the ongoing effort of reducing the dependency of the acceptance test-lib. The order of execution of the acceptance tests had to be updated to make sure that the `governance.test.js` would run after the state-sync acceptance tests to prevent the behavior described here #10574 ### Upgrade Considerations No upgrade considerations.

dckc added test Governance Governance flake flakey test labels Dec 16, 2024

LuqiPan assigned anilhelvaci Dec 16, 2024

anilhelvaci added a commit that referenced this issue Dec 17, 2024

chore: switch using waitForElectionResult, eliminate flakiness

28b4884

refs: #10708

anilhelvaci mentioned this issue Dec 17, 2024

test(a3p): extend governance test coverage for acceptance proposal #10555

Merged

mergify bot closed this as completed in #10555 Dec 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

z:acceptance governance fails/flakes: No quorum #10708

z:acceptance governance fails/flakes: No quorum #10708

dckc commented Dec 16, 2024

dckc commented Dec 16, 2024 •

edited

Loading

dckc commented Dec 16, 2024

anilhelvaci commented Dec 17, 2024 •

edited

Loading

Jorge-Lopes commented Dec 17, 2024 •

edited

Loading

z:acceptance governance fails/flakes: No quorum #10708

z:acceptance governance fails/flakes: No quorum #10708

Comments

dckc commented Dec 16, 2024

dckc commented Dec 16, 2024 • edited Loading

dckc commented Dec 16, 2024

anilhelvaci commented Dec 17, 2024 • edited Loading

Investigation

Param Change Tx

Voting Duration

Thoughts

Solution

Jorge-Lopes commented Dec 17, 2024 • edited Loading

dckc commented Dec 16, 2024 •

edited

Loading

anilhelvaci commented Dec 17, 2024 •

edited

Loading

Jorge-Lopes commented Dec 17, 2024 •

edited

Loading