coral-xyz / anchor

⚓ Solana Sealevel Framework
https://anchor-lang.com
Apache License 2.0
3.52k stars 1.3k forks source link

lang: Feature Request for absorbing excess demand for accounts #1866

Open ngundotra opened 2 years ago

ngundotra commented 2 years ago

Problem:

Excess demand for writing to program accounts incentivizes transaction spamming. This transfers economic demand away from highly requested dApp/program usage to excessive demand for network bandwidth.

Historically, bandwidth demand from bots has incurred so much network traffic that Solana consensus is forced to restart. Examples of this:

Fix

Metaplex recently introduced a botting fee that will charge bots for incorrect transactions, rather than failing the transaction.

Pros:

Cons:

Request:

Allow new program developers to specify the conditions under which they charge bots, and enable a toggle to turn these fees on and off.

In practice, this could look something like the following:

// anchor_lang::prelude::{FeeConfig, ErrorTracker}

#[derive(Accounts)]
// Instead of failing tx on these errors, charges flat base_fee
#[on_error(
  [
    SpamError::BeforeAvailable,
    SpamError::NoLongerAvailable
  ],
  base_fee = UNAVAILABLE_TIME_TX_FEE,
  pay_to = pda_from_account(account.key().as_ref())
)]
struct NormalContext<'info> {
  ...
  #[account(...)]
  account: Account<'info, HighlyRequestedAccount>,
  fee_config: Account<'info, FeeConfig>,
  token_program: AccountInfo<'info>
}

#[derive(Accounts)]
// Instead of failing tx, charges base fee, 
// multiplied by exponential amount for each failed attempt in error_tracker
#[on_error(
  SpamError::UnqualifiedRequest,
  base_fee = UNQUALIFIED_TX_FEE,
  exponential = 10,
  pay_to = pda_from_account(account.key().as_ref())
)]
struct ExponentialContext<'info> {
  ...
  #[account(...)]
  account: Account<'info, HighlyRequestedAccount>,
  #[account(track = payer)]
  error_tracker: Account<'info, ErrorTracker>,
  fee_config: Account<'info, FeeConfig>,
  token_program: AccountInfo<'info>
}

Welcome to all feedback on this. I do not believe all of this functionality should live in Anchor, but I do believe this is the right place to start the discussion.

ngundotra commented 2 years ago

Planning to implement the simple flat fee bot tax by adding a handle_error function to the Accounts trait

pub trait Accounts<'info>: ToAccountMetas + ToAccountInfos<'info> + Sized {
  ...
  fn handle_error() -> Result<()>;
}

So that when the codegen'd dispatch actually invokes an instruction, it will call

match #invoke_space.#instruction(#args) {
  Ok(()) => (),
  Err(err) => ctx.accounts.handle_error(err),
}

Handle error will be generated from the derive Accounts attribute

#[derive(Accounts)]
#[bot_tax(errors=[ErrorCode::BotTaxableError], lamport_fee=1, pay_to=pay_to)]
pub struct MyContext {
  ...
  #[account(mut)]
  /// CHECK: this account is not read from
  pay_to: AccountInfo<'info>
}

and have default implementation of

fn handle_error() -> Result<()> {
  Ok(())
}
armaniferrante commented 2 years ago

Will we have any guarantees on whether state transitions will have been rolled back in the event of an error? Probably not? This might be an added footgun, since we are effectively turning an aborted transaction into a successful one + fee.

ngundotra commented 2 years ago

Is there any way not to make this into a footgun?

What if we prevented footgunning by checking if this an inner instruction or first/last instruction? User can specify whether the instruction is okay to be CPI'd into, okay to be first instruction, or okay to be last instruction. Otherwise, the instruction will actually fail immediately.

For example:

#[bot_tax(
   errors = [ErrorCode::BotTaxableError], 
   accessible_via = [InstructionContext::Last],
   lamport_fee=1, 
   pay_to=pay_to
)]

MPL's candy machine executes more complicated forms of this check here: https://github.com/metaplex-foundation/metaplex-program-library/blob/master/candy-machine/program/src/lib.rs#L62-L93

ngundotra commented 2 years ago

Maybe place this check in separate anchor-extension ?

As I'm publicly exposing thoughts on this, I realize this may not be the best place to put this check.

I would prefer to expose a bot_tax crate that extends anchor and adds this trait as an extension to derive(Accounts). That way there would be a dedicated space to throw arbitrarily complex anti-bot logic.

@armaniferrante is it possible to create an crate that extends this macro, or do I need to submit a PR that allows extensions to be configured for derive(Accounts)?

armaniferrante commented 2 years ago

Is there any way not to make this into a footgun?

I don't think this prevents the footgun. What I was originally referring to was the case where, you perform some instruction, change some state, return an error, and instead of aborting the entire transaction, the error gets thrown away and replaced with a fee. The state transition will persist leaving the program in an intermediate, undesirable state, instead of rolling back.

As I'm publicly exposing thoughts on this, I realize this may not be the best place to put this check.

Extending the macro might be challenging/impossible at the moment. Though we can put this feature behind a feature flag, to have a similar effect.

ngundotra commented 2 years ago

you perform some instruction, change some state, return an error, and instead of aborting the entire transaction

Yes, I think this will be a part of risk of using this, but it's also just an issue with being a good smart contract dev. This is why audits are great!

Extending the macro might be challenging/impossible at the moment.

Okay then maybe I can refactor this into an anchor-fees crate? Instead of intercepting errors, the crate can expose a bunch of mechanisms / structs that make it easy to charge different accounts fees & track behavior.

for example:

fn charge_fee_native(payer, pay_to, fee) -> anchor_lang::Result<()>;

fn charge_expo_fee_native(payer, pay_to, fee_tracker) -> anchor_lang::Result<()>;

/// PDA for tracking offending bots
#[derive(AnchorDeserialize, AnchorSerialize, Clone, Copy, Default, Debug)]
struct FeeTracker {
  tracking: Pubkey,
  count: u32,
  base: u32,
  rate: u8,
}
jarry-xiao commented 2 years ago

I think this is in general a really hard to to generalize on a DApp level, but I do think that there are good use cases for similar macros for instruction checking (whitelisted programs, last ix in current tx). Probably best to tackle this hard game/graph theory problem on a validator level