Agoric / agoric-sdk

monorepo for the Agoric Javascript smart contract platform
Apache License 2.0
328 stars 207 forks source link

feeDistributor vat is not ugpradable #8729

Open warner opened 10 months ago

warner commented 10 months ago

While looking at https://github.com/Agoric/agoric-sdk/pull/8499#issuecomment-1842457715 . and thinking about how v28-feeDistributor would react under upgrade, we noticed that v28-feeDistributor is not ugpradable.

v23-feeDistributor has code which talks to the timer service and creates a notifier during startDistributing. However this function is called only from makeFeeDistributor, which creates ephemeral creatorFacet and publicFacet objects (using Far()). So this contract instance is not upgradable: if we were to deploy any new version of v23-feeDistributor, these facets would be abandoned, making them unavailable for clients.

Since we can't upgrade v23-feeDistributor, we'll need to replace it, somehow, by convincing any clients to use a different vat, and then terminate the old one.

I didn't find any other tickets describing our plan to replace this code, so I figured I should start a new one.

cc @Chris-Hibbert

dckc commented 10 months ago

Um... did we miss this one in the bulldozer planning somehow? No... we decided it was OK:

"cron job" with no important state. can terminated and replaced at will. -- https://github.com/Agoric/agoric-sdk/issues/5795

https://github.com/Agoric/agoric-sdk/blob/d3ddae6fa38b289b415d9f1bbbbd83338ad4aed3/packages/builders/scripts/vats/restart-vats.js#L7-L9

I think there are no clients.

Chris-Hibbert commented 6 months ago

The feeDistributor may only be used by PSM and vat-bank.

dckc commented 6 months ago

feeDistributor collects from vaultFactory and sends to reserve; so it's a client of those 2.

https://github.com/Agoric/agoric-sdk/blob/1749514b5b69a3112d60293ffc141885e4ff2edd/packages/inter-protocol/src/proposals/econ-behaviors.js#L500-L502

PSM could generate fees, but it doesn't, so we didn't write it up, AFAICT.

feeDistributor 's only client is the time service, I think.

Chris-Hibbert commented 6 months ago

The feeDistributor does all the work, and knows both the payers and payees. Neither of those has any knowledge of the feeDistributor.

The bootstrapSpace has creatorFacets for the payers, which have a makeCollectFeesInvitation() method. It also knows how to make depositFacets for the recipients of the funds.

To replace (not upgrade) the feeDistributor, just create a new one, and register it as the feeDistributor

  feeDistributorKit.resolve(
    harden({ ...instanceKit, label: 'feeDistributor' }),
  );
  feeDistributorP.resolve(instanceKit.instance);

The only current provider of fees is vaultFactory, and the recipients are current configured as

      keywordShares: {
        RewardDistributor: 0n,
        Reserve: 1n,
      }
LuqiPan commented 2 weeks ago

@Chris-Hibbert, this looks to me a duplicate of #10393 and as such, should it be closed?

Chris-Hibbert commented 2 weeks ago

@LuqiPan It has details that weren't reproduced in #10393

@anilhelvaci, PTAL at this and verify that it doesn't raise any issues that we might have missed in #10453.