SubstrateChess / pallet-chess

The Unlicense
11 stars 6 forks source link

Clear expired message #15

Closed lana-shanghai closed 1 year ago

bernardoaraujor commented 1 year ago

that's awesome, thanks a lot for this contribution! 🙏

I think we can simplify the solution so that it does not require a third party. Something like: clear_expired_match is actually called claim_victory and can only be called by the player who's waiting for their opponent's move.

That way we wouldn't need to think about incentivizing someone to do this "janitor" work. That would also simplify the execution complexity of this new extrinsic (no need to check for time_since_expiry).

During my flight back home I started drafting a solution for this "janitor" work based on Offchain Workers. I'll prepare a new PR after this one is merged, but the idea goes like this:


The OCW will do the same logic that's currently implemented by on_initialize, basically scanning all on-chain matches and checking if they were "abandoned". The criteria for a match being classified as "abandoned" is something like:

fn check_abandoned(block_number: T::BlockNumber, m: Match<T>) -> bool {
    let diff = block_number - m.last_move;
    let abandoned: bool = match m.style {
        MatchStyle::Bullet => diff > T::BulletPeriod::get() * 10u32.into(),
        MatchStyle::Blitz => diff > T::BlitzPeriod::get() * 10u32.into(),
        MatchStyle::Rapid => diff > T::RapidPeriod::get() * 10u32.into(),
        MatchStyle::Daily => diff > T::DailyPeriod::get() * 10u32.into(),
    };

    abandoned
}

which means the rightful winner has 9*T::_Period::get() to claim their victory due to opponent inactivity. If they don't, a janitor OCW calls janitor_job, which is a feeless extrinsic. An on-chain StorageMap keeps track of the authorized janitor keys (added/removed by root/governance), and a SignedExtension makes sure that janitor_job can only enter the tx pool if the signature comes from an authorized janitor key.

This solution offloads the blockspace consumed by on_initialize in the current solution, and also automates the janitor work so that the cleaning up the pallet's storage doesn't depend on some third party incentives.

bernardoaraujor commented 1 year ago

However, after writing the comments above I realized that this OCW solution leaves room for attack vectors if janitor keys are stolen, or some janitor becomes malicious.

We can partially mitigate that by making sure that janitor_job calls check_abandoned before cleaning up the match from storage. That would prevent the malicious janitor from erasing ongoing matches.

But there's still room for spam, since janitor_job would be feeless. The malicious janitor could spam the chain by calling janitor_job without paying any fees.


The solution that is proposed on this PR doesn't have the issues that Janitor OCWs introduce. However it also introduces a new problem of its own nature:

when a player thinks they're about to lose, they would have incentives to refuse to play and still get a percentage of their bet back... this is arguably a very narrow attack vector, since winner can just claim their victory within the incentive period

~I made some suggestions on the code that improve this aspect a little bit (if loser is the origin for clear_expired_match, then chess_match.clear_expired_bet(&winner, &loser) is never called)~

but an informed loser could still attack their opponent by:

  1. refusing to play
  2. calling clear_expired_match from a different account that they also control