From c60728eaa88d4c93f32b99bfd3adfb0a13d0aea1 Mon Sep 17 00:00:00 2001 From: zakir <80246097+zakir-code@users.noreply.github.com> Date: Fri, 2 Feb 2024 17:28:36 +0800 Subject: [PATCH] refactor: move tryAttestion from abci to claimMsg (#188) Co-authored-by: fx0x55 <80245546+fx0x55@users.noreply.github.com> --- x/crosschain/keeper/abci.go | 57 -------------------------- x/crosschain/keeper/attestation.go | 18 +++----- x/crosschain/keeper/msg_server_test.go | 4 ++ 3 files changed, 9 insertions(+), 70 deletions(-) diff --git a/x/crosschain/keeper/abci.go b/x/crosschain/keeper/abci.go index 55bdbe8a9..be1eb0704 100644 --- a/x/crosschain/keeper/abci.go +++ b/x/crosschain/keeper/abci.go @@ -13,7 +13,6 @@ import ( func (k Keeper) EndBlocker(ctx sdk.Context) { signedWindow := k.GetSignedWindow(ctx) k.slashing(ctx, signedWindow) - k.attestationTally(ctx) k.cleanupTimedOutBatches(ctx) k.createOracleSetRequest(ctx) k.pruneOracleSet(ctx, signedWindow) @@ -124,62 +123,6 @@ func (k Keeper) batchSlashing(ctx sdk.Context, oracles types.Oracles, signedWind return hasSlash } -// Iterate over all attestations currently being voted on in order of nonce and -// "Observe" those who have passed the threshold. Break the loop once we see -// an attestation that has not passed the threshold -func (k Keeper) attestationTally(ctx sdk.Context) { - type attClaim struct { - *types.Attestation - types.ExternalClaim - } - attMap := make(map[uint64][]attClaim) - // We make a slice with all the event nonces that are in the attestation mapping - var nonces []uint64 - k.IterateAttestationAndClaim(ctx, func(att *types.Attestation, claim types.ExternalClaim) bool { - if v, ok := attMap[claim.GetEventNonce()]; !ok { - attMap[claim.GetEventNonce()] = []attClaim{{att, claim}} - nonces = append(nonces, claim.GetEventNonce()) - } else { - attMap[claim.GetEventNonce()] = append(v, attClaim{att, claim}) - } - return false - }) - // Then we sort it - sort.Slice(nonces, func(i, j int) bool { - return nonces[i] < nonces[j] - }) - - // This iterates over all nonces (event nonces) in the attestation mapping. Each value contains - // a slice with one or more attestations at that event nonce. There can be multiple attestations - // at one event nonce when Oracles disagree about what event happened at that nonce. - for _, nonce := range nonces { - // This iterates over all attestations at a particular event nonce. - // They are ordered by when the first attestation at the event nonce was received. - // This order is not important. - for _, att := range attMap[nonce] { - // We check if the event nonce is exactly 1 higher than the last attestation that was - // observed. If it is not, we just move on to the next nonce. This will skip over all - // attestations that have already been observed. - // - // Once we hit an event nonce that is one higher than the last observed event, we stop - // skipping over this conditional and start calling tryAttestation (counting votes) - // Once an attestation at a given event nonce has enough votes and becomes observed, - // every other attestation at that nonce will be skipped, since the lastObservedEventNonce - // will be incremented. - // - // Then we go to the next event nonce in the attestation mapping, if there is one. This - // nonce will once again be one higher than the lastObservedEventNonce. - // If there is an attestation at this event nonce which has enough votes to be observed, - // we skip the other attestations and move on to the next nonce again. - // If no attestation becomes observed, when we get to the next nonce, every attestation in - // it will be skipped. The same will happen for every nonce after that. - if nonce == k.GetLastObservedEventNonce(ctx)+1 { - k.TryAttestation(ctx, att.Attestation, att.ExternalClaim) - } - } - } -} - // cleanupTimedOutBatches deletes batches that have passed their expiration on Ethereum // keep in mind several things when modifying this function // A) unlike nonces timeouts are not monotonically increasing, meaning batch 5 can have a later timeout than batch 6 diff --git a/x/crosschain/keeper/attestation.go b/x/crosschain/keeper/attestation.go index 74c1f3365..e106c0234 100644 --- a/x/crosschain/keeper/attestation.go +++ b/x/crosschain/keeper/attestation.go @@ -43,9 +43,12 @@ func (k Keeper) Attest(ctx sdk.Context, oracleAddr sdk.AccAddress, claim types.E // Add the oracle's vote to this attestation att.Votes = append(att.Votes, oracleAddr.String()) - k.SetAttestation(ctx, claim.GetEventNonce(), claim.ClaimHash(), att) + if !att.Observed && claim.GetEventNonce() == k.GetLastObservedEventNonce(ctx)+1 { + k.TryAttestation(ctx, att, claim) + } + ctx = ctx.WithGasMeter(gasMeter) k.SetLastEventNonceByOracle(ctx, oracleAddr, claim.GetEventNonce()) k.SetLastEventBlockHeightByOracle(ctx, oracleAddr, claim.GetBlockHeight()) @@ -57,10 +60,6 @@ func (k Keeper) Attest(ctx sdk.Context, oracleAddr sdk.AccAddress, claim types.E // and has not already been marked Observed, then calls processAttestation to actually apply it to the state, // and then marks it Observed and emits an event. func (k Keeper) TryAttestation(ctx sdk.Context, att *types.Attestation, claim types.ExternalClaim) { - if att.Observed { - // We panic here because this should never happen - panic("attempting to process observed attestation") - } // If the attestation has not yet been Observed, sum up the votes and see if it is ready to apply to the state. // This conditional stops the attestation from accidentally being applied twice. // Sum the current powers of all validators who have voted and see if it passes the current threshold @@ -82,14 +81,7 @@ func (k Keeper) TryAttestation(ctx sdk.Context, att *types.Attestation, claim ty if attestationPower.LT(requiredPower) { continue } - // If the power of all the validators that have voted on the attestation is higher or equal to the threshold, - // process the attestation, set Observed to true, and break - lastEventNonce := k.GetLastObservedEventNonce(ctx) - // this check is performed at the next level up so this should never panic - // outside of programmer error. - if claim.GetEventNonce() != lastEventNonce+1 { - panic("attempting to apply events to state out of order") - } + k.SetLastObservedEventNonce(ctx, claim.GetEventNonce()) k.SetLastObservedBlockHeight(ctx, claim.GetBlockHeight(), uint64(ctx.BlockHeight())) diff --git a/x/crosschain/keeper/msg_server_test.go b/x/crosschain/keeper/msg_server_test.go index ea667a4ec..d02cd1c3b 100644 --- a/x/crosschain/keeper/msg_server_test.go +++ b/x/crosschain/keeper/msg_server_test.go @@ -725,6 +725,10 @@ func (suite *KeeperTestSuite) TestClaimMsgGasConsumed() { execute: func(claimMsg types.ExternalClaim) (minGas, maxGas, avgGas uint64) { msg, ok := claimMsg.(*types.MsgSendToExternalClaim) suite.True(ok) + suite.Require().NoError(suite.Keeper().StoreBatch(suite.ctx, &types.OutgoingTxBatch{ + BatchNonce: msg.BatchNonce, + TokenContract: msg.TokenContract, + })) for i, oracle := range suite.oracleAddrs { eventNonce := suite.Keeper().GetLastEventNonceByOracle(suite.ctx, oracle) msg.EventNonce = eventNonce + 1