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

AIP #61: AdEx v5: significantly cheaper channels #377

Open
50 of 53 tasks
elpiel opened this issue Feb 22, 2021 · 2 comments · Fixed by #433, #443, #437, #473 or #477
Open
50 of 53 tasks

AIP #61: AdEx v5: significantly cheaper channels #377

elpiel opened this issue Feb 22, 2021 · 2 comments · Fixed by #433, #443, #437, #473 or #477
Assignees
Labels
tracking Tracking issue for a bigger task
Milestone

Comments

@elpiel
Copy link
Member

elpiel commented Feb 22, 2021

Tracking issue

For the changes regarding AIP#61: AdEx v5: significantly cheaper channels (full details here)

Change log

pub struct Deposit<T> {
    pub total: T,
    pub still_on_create2: T,
}

impl Deposit<UnifiedNum> {
   pub fn from_precision(precision: u8, deposit: Deposit<BigNum>) -> Option<Deposit<UnifiedNum>> {... }
}

deposit into a channel and create a campaign make sure it doesn't work before that submit events verify that payments appear in NewState/ApproveState, incl. fees verify that /analytics returns stuff

Remove old V4 routes that are no longer needed:

Sentry Authentication required (AuthRequired middleware) routes

  • POST /v5/campaign (Auth is used to validate the campaign.creator)
  • POST /v5/campaign/:id/close (Auth is used to validate the campaign.creator)
  • POST /v5/channel/:id/spender/:addr
  • GET /v5/channel/:id/spender/:addr
  • POST /v5/channel/:id/spender/all
  • GET channel/:id/events-aggregates (v4)
  • POST channel/:id/validator-messages (v4)
  • POST /v5/channel/:id/validator-messages
  • /v5/channel/:id/pay
  • /v5/analytics/for-publisher
  • /v5/analytics/for-advertiser
  • /v5/analytics/for-admin
@elpiel elpiel added the tracking Tracking issue for a bigger task label Feb 22, 2021
@elpiel elpiel changed the title AIP#61 Tracking issue AIP#61 AdEx v5: significantly cheaper channels Feb 22, 2021
@elpiel elpiel changed the title AIP#61 AdEx v5: significantly cheaper channels AIP #61 AdEx v5: significantly cheaper channels Mar 8, 2021
@elpiel elpiel pinned this issue Mar 25, 2021
@elpiel
Copy link
Member Author

elpiel commented Oct 4, 2021

Here are a few questions and remarks on the current state of V5.
@Ivshti should take a look at the pending questions and we will discuss them together.

  • PAY channel event (POST /v5/channel/:id/pay)
    • how will this event be handled in analytics and accounting?
    • How will this affect campaigns' remaining budget?
  • A Channel is created implicitly when creating a Campaign and we don't have a route for creating channels. Validate if the following statement is the desirable outcome:
    • GET/POST /v5/channel/:id/spender/* will all return 404 if there is no Campaign with the given Channel.id. Updating the Spendable amount (POST /v5/channel/:id/spender/:addr) will not be executable in this case until a Campaign is created with the given Channel.
  • On Campaign creation (POST /v5/campaign) in this case we perform:
    • Channel insertion in DB (if it does not exist)
    • Updating Spendable amount in the DB using the Adapter.
  • On Campaign modification (POST /v5/campaign/:id) only when the budget is modified we also update the Spendable amount in the DB using the Adapter.
  • How should we perform the GET /v5/channel/:id/get-leaf? Are these the correct steps to produce the given leaf:
    1. Generating a MerkleTree by getting the balance leaf of the spender/earner (see
      pub fn get_balance_leaf(
      is_spender: bool,
      acc: &Address,
      amnt: &BigNum,
      ) -> Result<[u8; 32], BalanceLeafError> {
      let address = Token::Address(EthAddress::from_slice(acc.as_bytes()));
      let amount = Token::Uint(
      U256::from_dec_str(&amnt.to_str_radix(10))
      .map_err(|_| BalanceLeafError("Failed to parse amt".into()))?,
      );
      let tokens = if is_spender {
      vec![Token::String("spender".into()), address, amount]
      } else {
      vec![address, amount]
      };
      let encoded = encode(&tokens).to_vec();
      let mut result = Keccak::new_keccak256();
      result.update(&encoded);
      let mut res: [u8; 32] = [0; 32];
      result.finalize(&mut res);
      Ok(res)
      }
      )
    2. making a singable state root for the Channel ( see
      pub fn get_signable_state_root(channel_id: &[u8], balance_root: &[u8; 32]) -> [u8; 32] {
      let tokens = [
      Token::FixedBytes(channel_id.to_vec()),
      Token::FixedBytes(balance_root.to_vec()),
      ];
      let encoded = encode(&tokens).to_vec();
      let mut result = Keccak::new_keccak256();
      result.update(&encoded);
      let mut res: [u8; 32] = [0; 32];
      result.finalize(&mut res);
      res
      }
      )
    3. And finally hex encoding the result like we do with the Channel events
    • What should happen if a request does not include a spender or earner query parameter?
    • 400
  • How should we be generating the SpenderLeaf merkle proof for GET /v5/channel/:id/spender/:addr & GET /v5/channel/:id/spender/all? We will drop this value from the response. API users will be forced to use get-leaf
  • Should we keep the current behavior on GET /v5/campaign/list query parameter - isLeader?
    • query.validator is passed - filters campaign.channel.leader = query.validator
    • no query.validator but is an authenticated call (we have Bearer auth token) - filter by Campaign.channel.leader = Auth.uid
    • We will not keep this behavior. Instead use validator=0x... (for returning campaigns that have the validator as either leader or follower) & leader=0x... (returns only campaigns that the Channel.leader = 0x...)

@Ivshti
Copy link
Member

Ivshti commented Oct 4, 2021

  • PAY event; the PAY event is outside the campaign, it is not associated with any campaign, and it affects Accounting directly without doing anything about the analytics; see "please note that we can't assume totalSpent equals the sum of all campaign spendings, because a spending can occur outside of campaign (cases like v4 CLOSE or PAY)"
  • re non-existing channels, this behavior makes sense
  • on campaign creation: makes sense
  • when to update Spendable: also makes sense
  • get-leaf: that's correct; if it doesn't conatin spender/earner, it's a 400 bad request
  • sounds like the same procedure like get-leaf would be sufficient; actually, let's not return a leaf in this route for now, and force users to always call get-leaf
  • this behavior is used by the validator when running through the campaigns to update them, so you decide whether to keep it depending on whether it's needed in the validator - I am pretty sure it is, so we'll need to keep it

@elpiel elpiel linked a pull request Oct 4, 2021 that will close this issue
@elpiel elpiel changed the title AIP #61 AdEx v5: significantly cheaper channels AIP #61: AdEx v5: significantly cheaper channels Jan 19, 2022
@elpiel elpiel linked a pull request Feb 21, 2022 that will close this issue
@elpiel elpiel linked a pull request Mar 24, 2022 that will close this issue
3 tasks
@elpiel elpiel linked a pull request Apr 13, 2022 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment