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

test/integ: update testcontainers and deps, workaround expected occasional missed approve refund #618

Merged
merged 2 commits into from
Mar 28, 2024

Conversation

jchappelow
Copy link
Member

@jchappelow jchappelow commented Mar 28, 2024

This works around repeated failures in several integration tests that were doing balance checks that were foiled by missed refunds due to late approvals.

The slowest <1/3 of validators will routinely miss having their approval
votes included in a block that contains the resolution. As a result, they
will not get the refund expected when placing a valid vote. To deal with
this possibility in the tests where we check expected balance changes,
sleep a little after deposit transactions are resolved so that the late
approval can be mined in the subsequent block.  Without the delay, the
balance will be less the gas consumed by the approve txn.

This also updates test/go.mod to use the latest test containers and docker package dependencies, which really wasn't required after all, but should support Docker 26, which came out last week.

The slowest <1/3 of validators will routinely miss having their approval
votes included in a block that contains the resolution. As a result, they
will not get the refund expected when placing a valid vote. To deal with
this possibility in the tests where we check expected balance changes,
sleep a little after deposit transactions are resolved so that the late
approval can be mined in the subsequent block.  Without the delay, the
balance will be less the gas consumed by the approve txn.
@jchappelow jchappelow changed the title ci: update testcontainers and friends test/integ: update testcontainers and deps, workaround expected occasional missed approve refund Mar 28, 2024
@jchappelow
Copy link
Member Author

jchappelow commented Mar 28, 2024

@charithabandi confirmed the late votes thing is common.

With diff not in this PR:

diff --git a/internal/txapp/routes.go b/internal/txapp/routes.go
index ce9a9a11..7d1039fc 100644
--- a/internal/txapp/routes.go
+++ b/internal/txapp/routes.go
@@ -710,6 +710,10 @@ func (v *validatorVoteIDsRoute) Execute(ctx TxContext, router *TxApp, tx *transa
                }
        }
 
+       if tooLate := len(approve.ResolutionIDs) - len(ids); tooLate > 0 {
+               router.Log().Warnf("vote contains %d resolution IDs that are already done. too late, no refund!", tooLate)
+       }
+
        err = tx2.Commit(ctx.Ctx)
        if err != nil {
                return txRes(spend, transactions.CodeUnknownError, err)
diff --git a/internal/txapp/txapp.go b/internal/txapp/txapp.go
index dec6ffa0..f730f436 100644
--- a/internal/txapp/txapp.go
+++ b/internal/txapp/txapp.go
@@ -107,6 +107,10 @@ type TxApp struct {
        resTypes            []string
 }
 
+func (r *TxApp) Log() *log.Logger {
+       return &r.log
+}
+
 // GenesisInit initializes the TxApp. It must be called outside of a session,
 // and before any session is started.
 // It can assign the initial validator set and initial account balances.
2024-03-28T21:06:40.222Z        info    kwild.tx-service        v1/broadcast.go:79      broadcast transaction   {"rpc": "Broadcast", "PayloadType": "transfer", "from": "c89d42189f0
450c2b2c3c61f58ec5d628176a1e7", "TxHash": "d8de296e09b2c8cf537eab055ca8f2eaf4bd8471ef5110f386d123c4755d1de7", "sync": 1, "nonce": 1}
2024-03-28T21:06:40.222Z        info    kwild.grpc-server       call success    {"method": "tx.TxService/Broadcast", "elapsed": "9.955ms", "addr": "127.0.0.1:44300", "code": "OK"}
2024-03-28T21:06:40.588Z        info    kwild.cometbft  consensus/state.go:1715 finalizing commit of block      {"module": "consensus", "height": 5, "hash": "9D2500E8463070BB2F1D8F
1CDEAA7C329A4A8887B2A25B41047E43D86EB1F832", "root": "617F0FC8471C4035D1A307F32ADA4EFB02E3650CCE738E3B919BBB4005134C3F", "num_txs": 1}
2024-03-28T21:06:40.618Z        warn    kwild.tx-router txapp/routes.go:714     vote contains 1 resolution IDs that are already done. too late, no refund!
2024-03-28T21:06:40.638Z        info    kwild.pg        pg/repl.go:244  Commit hash e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855, seq 5, LSN 0/1C0D980 (29415808) delta 1144
2024-03-28T21:06:40.639Z        info    kwild.cometbft  state/execution.go:230  finalized block {"module": "state", "height": 5, "num_txs_res": 1, "num_val_updates": 0, "block_app_hash": "CC8BBE280D850D05781A63BECB98B435C0F4066850FD7E5C7433D9A48B3FBB9F"}
2024-03-28T21:06:40.66Z info    kwild.abci      abci/abci.go:186        Recheck {"recheck": true, "hash": "d8de296e09b2c8cf537eab055ca8f2eaf4bd8471ef5110f386d123c4755d1de7", "nonce": 1, "payloadType": "transfer", "sender": "c89d42189f0450c2b2c3c61f58ec5d628176a1e7"}
2024-03-28T21:06:41.98Z info    kwild.cometbft  consensus/state.go:1715 finalizing commit of block      {"module": "consensus", "height": 6, "hash": "F01A5FBF33C321EA8460575EBC8CDEB51CDD9DB8F95BB443C02D5D07707A9428", "root": "CC8BBE280D850D05781A63BECB98B435C0F4066850FD7E5C7433D9A48B3FBB9F", "num_txs": 1}
2024-03-28T21:06:42.027Z        info    kwild.pg        pg/repl.go:244  Commit hash e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855, seq 6, LSN 0/1C0E108 (29417736) delta 1112

@jchappelow jchappelow merged commit a9930db into kwilteam:main Mar 28, 2024
1 check passed
@jchappelow jchappelow deleted the tc-up branch March 28, 2024 21:16
@jchappelow jchappelow added this to the v0.8.0 milestone May 2, 2024
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.

2 participants