Enforce vote delay for proxy votes - #1545
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a261674935
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| boolean queuedProxyVoteAlreadyRecorded = ProxyVoteDelayCheck.isQueuedVoteAlreadyRecorded(event.isBungee(), | ||
| event.getTime(), user.getTime(voteSite)); | ||
| if (voteSite.isWaitUntilVoteDelay() && !queuedProxyVoteAlreadyRecorded && !user.canVoteSite(voteSite)) { | ||
| if (voteSite.isWaitUntilVoteDelay() && !user.canVoteSite(voteSite)) { |
There was a problem hiding this comment.
Restore the exact-timestamp exception for queued proxy votes. In single-server proxy mode, an offline vote is placed in OnlineCache and a VoteUpdate records its timestamp on the backends (VotingPluginProxy.java:1867-1878); when the player later joins, checkOnlineVotes sends that cached vote and immediately removes it (VotingPluginProxy.java:536-563). Because canVoteSite now sees the same timestamp and returns false, this condition rejects the legitimate cached vote, so its normal rewards/totals are never processed and the cache entry is lost whenever WaitUntilVoteDelay is enabled.
Useful? React with 👍 / 👎.
|
@codex address the unresolved P1 review feedback. Preserve legitimate delivery of already-recorded queued proxy votes without reintroducing a general timestamp-equality replay bypass: bind the exception to the actual queued-vote flow/metadata and ensure stale or forged proxy messages remain rejected. Add regression coverage for legitimate queued delivery and replay/stale cases, then run relevant checks. |
Summary
Testing
|
|
Implemented the Codex finding directly: legitimate queued proxy votes with the recorded timestamp are accepted again, while direct/stale messages remain subject to the delay check. @codex review the new commit. |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review the current head after the queued-proxy metadata fix and regression tests. Please verify that only identified VoteOnline deliveries receive the timestamp exception, while ordinary, stale, forged, or unidentified proxy votes remain delay-protected, and confirm whether this PR is ready. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a71c470af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| boolean identifiedQueuedDelivery = VotingPluginWire.SUB_VOTE_ONLINE.equals(msg.getSubChannel()) | ||
| && v.voteId != null; |
There was a problem hiding this comment.
A VoteOnline subchannel plus any non-null wire-supplied UUID does not prove that this message is the current delivery of an OnlineCache entry. After processedWireVotes expires the UUID in 30 minutes (or after a backend restart), replaying a previously accepted VoteOnline reserves the ID again; its timestamp still equals LastVotes, so this classification causes the listener to skip WaitUntilVoteDelay and process duplicate rewards/totals. A forged VoteOnline with a fresh UUID and the stored timestamp has the same result. Bind the exception to actual queue-delivery provenance rather than these forgeable fields.
Useful? React with 👍 / 👎.
| user.bungeeVotePluginMessaging(service, v.time, text, setTotalsOnBackend, v.wasOnline, v.broadcast, v.num, | ||
| identifiedQueuedDelivery); |
There was a problem hiding this comment.
When PerServerPoints is enabled and an ordinary, stale, or unidentified proxy vote fails WaitUntilVoteDelay, bungeeVotePluginMessaging only fires the event and returns normally after PlayerVoteListener exits early. The handler then unconditionally executes the user.addPoints(...) block immediately below, so a delay-rejected message still awards points; forged messages with fresh vote IDs can repeat this even though their normal rewards and totals are blocked. Propagate whether the event was accepted and grant per-server points only after successful processing.
Useful? React with 👍 / 👎.
| public void bungeeVotePluginMessaging(String service, long time, VoteTotalsSnapshot text, boolean setTotals, | ||
| boolean wasOnline, boolean broadcast, int num, boolean queuedProxyVote) { |
There was a problem hiding this comment.
Changing this public method's descriptor from seven parameters to eight removes the method that already-compiled integrations link against. The project publishes the plugin as a Maven dependency and generates API documentation, so any downstream plugin calling the former bungeeVotePluginMessaging(..., int) overload will fail with NoSuchMethodError after upgrading even though its source remains valid against the prior release. Keep the old overload and delegate it to the new method with queuedProxyVote set to false.
Useful? React with 👍 / 👎.
1 participant
Motivation
WaitUntilVoteDelaywhen the proxy message timestamp equalled the stored last-vote time, which can be replayed or forged to gain duplicate rewards.Description
ProxyVoteDelayCheckand its unit testProxyVoteDelayCheckTestto eliminate the timestamp-equality bypass.PlayerVoteListenerto stop consulting the removed helper and to apply theWaitUntilVoteDelaygate uniformly (voteSite.isWaitUntilVoteDelay()+user.canVoteSite(voteSite)), rejecting ineligible votes before reward/total processing.VotingPlugin.BypassWaitUntilVoteDelay, and configuredWaitUntilVoteDelayrewards.Testing
git diff --check,git show --check --stat, and searched the source forProxyVoteDelayCheckandqueuedProxyVoteAlreadyRecorded, which confirmed the helper and references were removed.Enforce vote delay for proxy votesand the working tree is clean after the change.mvn -q -DskipTests compileandmvn -q test, but both were blocked by external dependency resolution failing with HTTP 403 from Maven Central formaven-resources-plugin:3.3.1, so the project build/tests could not be executed in this environment.Codex Task