Skip to content

Commit

Permalink
Simplify the executeGrainIntegration function interface
Browse files Browse the repository at this point in the history
Now that all intergration funcitonality has settled in the ledger, this
interface can be simplified and cleaned up. No information has been
lost; Config details can now simply be queried from the ledger directly.

Config inconsistencies are now tracked in the main/grain.js API file.
The main api will be tracking changes to the external currency config and
update the ledger dynamically before any integrations run.

test plan: unit tests have been updated
  • Loading branch information
topocount committed Nov 1, 2021
1 parent d49e5fd commit d6021e6
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 65 deletions.
18 changes: 15 additions & 3 deletions packages/sourcecred/src/api/main/grain.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
executeGrainIntegration,
type GrainIntegrationOutput,
} from "../../core/ledger/grainIntegration";
import deepEqual from "lodash.isequal";

export type GrainInput = {|
+credGraph: CredGraph,
Expand Down Expand Up @@ -67,6 +68,19 @@ export function executeGrainIntegrationsFromGrainInput(
const integrationCurrency = grainInput.currencyDetails.integrationCurrency;
const grainIntegration = grainInput.grainConfig.integration;
const results = [];
if (!ledger.accounting().enabled) {
const ledgerCurrency = ledger.accounting().currency;
if (!deepEqual(ledgerCurrency, integrationCurrency)) {
integrationCurrency
? ledger.setExternalCurrency(
integrationCurrency.chainId,
integrationCurrency.tokenAddress
? integrationCurrency.tokenAddress
: undefined
)
: ledger.removeExternalCurrency();
}
}
// track the latest ledger in the for-loop for the purposes of returning it
// at the top level, observing that any function may deep-copy
// the ledger (thus creating a new reference we'll need to track) and also
Expand All @@ -77,9 +91,7 @@ export function executeGrainIntegrationsFromGrainInput(
const result = executeGrainIntegration(
ledgerResult,
grainIntegration.function,
distribution,
integrationCurrency,
false
distribution
);
const {output, distributionCredTimestamp, ledger: nextLedger} = result;
ledgerResult = nextLedger;
Expand Down
28 changes: 14 additions & 14 deletions packages/sourcecred/src/core/ledger/grainIntegration.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// @flow

import {Ledger, type PayoutAddress} from "./ledger";
import {Ledger, type PayoutAddress, type AccountingStatus} from "./ledger";
import {type Distribution} from "./distribution";
import {getDistributionBalances} from "./distributionSummary/distributionSummary.js";
import {type Currency, type CurrencyKey, getCurrencyKey} from "./currency.js";
import {getDistributionBalances} from "./distributionSummary/distributionSummary";
import {type Currency, type CurrencyKey, getCurrencyKey} from "./currency";
import {type IdentityId} from "../identity";
import * as NullUtil from "../../util/null";
import {type TimestampMs} from "../../util/timestamp";
Expand Down Expand Up @@ -39,7 +39,7 @@ export type IntegrationConfig = {|
// is utilized to enforce the existence of token balances outside of the
// ledger. Importantly, Grain Receipts from allocations are still tracked,
// because some grain distribution strategies rely on this information.
accountingEnabled: boolean,
accounting: AccountingStatus,
// This enables a new ledger event where all distributions after this
// config is enabled should have a matching "Integration" event. If not,
// an interface should prompt admin interface users that they haven't
Expand Down Expand Up @@ -79,13 +79,13 @@ export function executeGrainIntegration(
ledger: Ledger,
integration: GrainIntegrationFunction,
distribution: Distribution,
currency: Currency,
processDistributions: boolean,
sink?: IdentityId,
// TODO (@topocount) setup accounting config in ledger and remove this config
// which is just for proof-of-concept
accountingEnabled?: boolean = false
sink?: IdentityId
): GrainIntegrationResult {
const currency = ledger.externalCurrency();
const {
enabled: accountingEnabled,
trackDistributions: processDistributions,
} = ledger.accounting();
const {payoutDistributions, payoutAddressToId} = buildDistributionIndexes(
ledger,
distribution,
Expand All @@ -97,9 +97,9 @@ export function executeGrainIntegration(
let result;
try {
result = integration(payoutDistributions, {
currency,
accountingEnabled,
accounting: ledger.accounting(),
processDistributions,
currency,
});
if (processDistributions) ledger.markDistributionExecuted(distribution.id);
} catch (e) {
Expand Down Expand Up @@ -129,7 +129,7 @@ export function executeGrainIntegration(
export function buildDistributionIndexes(
ledger: Ledger,
distribution: Distribution,
currencyId: CurrencyKey
currencyKey: CurrencyKey
): {
payoutDistributions: PayoutDistributions,
payoutAddressToId: PayoutAddressToId,
Expand All @@ -139,7 +139,7 @@ export function buildDistributionIndexes(
const balances = getDistributionBalances(distribution);
for (const [id, amount] of balances.entries()) {
const {payoutAddresses, identity} = ledger.account(id);
const address = payoutAddresses.get(currencyId);
const address = payoutAddresses.get(currencyKey);
if (!address) continue;
// need to allow for identities that have since been merged to still claim
// funds if accounts are merged between a grain distribution and a
Expand Down
83 changes: 40 additions & 43 deletions packages/sourcecred/src/core/ledger/grainIntegration.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ describe("src/core/ledger/grainIntegration", () => {
parseAddress("0x0000000000000000000000000000000000000002"),
currency.chainId
);
ledger.setExternalCurrency(buildCurrency("BTC").chainId);
});
const getmockIntegration: (?boolean) => GrainIntegrationFunction = (
returnTransfers = false,
Expand Down Expand Up @@ -105,8 +106,6 @@ describe("src/core/ledger/grainIntegration", () => {
ledger,
getmockIntegration(),
distribution,
currency,
true,
sink
);
const result = diffLedger(newLedger, Ledger.parse(ledgerSnapshot));
Expand All @@ -116,35 +115,50 @@ describe("src/core/ledger/grainIntegration", () => {
"id": "000000000000000000000A",
"type": "MARK_DISTRIBUTION_EXECUTED",
},
"ledgerTimestamp": 10,
"uuid": "000000000000000000011A",
"ledgerTimestamp": 11,
"uuid": "000000000000000000012A",
"version": "1",
},
]);
});
it("doesn't transfer grain if processDistributions isn't set", () => {
ledger.disableIntegrationTracking();
const ledgerSnapshot = ledger.serialize();
const {ledger: newLedger} = executeGrainIntegration(
ledger,
getmockIntegration(true),
distribution,
currency,
false,
sink
);
const result = diffLedger(newLedger, Ledger.parse(ledgerSnapshot));
expect(result).toEqual([]);
});
it("doesn't transfer grain if a accounting isn't enabled", () => {
const ledgerSnapshot = ledger.serialize();
ledger.disableAccounting();
const {ledger: newLedger} = executeGrainIntegration(
ledger,
getmockIntegration(true),
distribution,
currency,
true,
sink,
false
sink
);
const result = diffLedger(newLedger, Ledger.parse(ledgerSnapshot));
expect(result).toContainEqual({
"action": {
"id": "000000000000000000000A",
"type": "MARK_DISTRIBUTION_EXECUTED",
},
"ledgerTimestamp": 13,
"uuid": "000000000000000000014A",
"version": "1",
});
});
it("doesn't transfer grain if a sink isn't set", () => {
const ledgerSnapshot = ledger.serialize();
const {ledger: newLedger} = executeGrainIntegration(
ledger,
getmockIntegration(true),
distribution
);
const result = diffLedger(newLedger, Ledger.parse(ledgerSnapshot));
expect(result).toEqual([
Expand All @@ -153,57 +167,40 @@ describe("src/core/ledger/grainIntegration", () => {
"id": "000000000000000000000A",
"type": "MARK_DISTRIBUTION_EXECUTED",
},
"ledgerTimestamp": 10,
"uuid": "000000000000000000011A",
"ledgerTimestamp": 11,
"uuid": "000000000000000000012A",
"version": "1",
},
]);
});
it("doesn't transfer grain if a sink isn't set", () => {
it("does not mark a distribution as executed if Integration tracking is disabled", () => {
const ledgerSnapshot = ledger.serialize();
ledger.disableIntegrationTracking();
const {ledger: newLedger} = executeGrainIntegration(
ledger,
getmockIntegration(true),
getmockIntegration(),
distribution,
currency,
true
sink
);
const result = diffLedger(newLedger, Ledger.parse(ledgerSnapshot));
expect(result).toEqual([
{
"action": {
"id": "000000000000000000000A",
"type": "MARK_DISTRIBUTION_EXECUTED",
"type": "DISABLE_GRAIN_INTEGRATION",
},
"ledgerTimestamp": 10,
"uuid": "000000000000000000011A",
"ledgerTimestamp": 11,
"uuid": "000000000000000000012A",
"version": "1",
},
]);
});
it("Throws if the ledger does not have integrations enabled when it's expected to", () => {
ledger.disableIntegrationTracking();
const thunk = () =>
executeGrainIntegration(
ledger,
getmockIntegration(),
distribution,
currency,
true,
sink
);
expect(thunk).toThrow("integration tracking not enabled");
});
it("updates the ledger when balances are returned by the integration", () => {
const ledgerSnapshot = ledger.serialize();
const {ledger: newLedger} = executeGrainIntegration(
ledger,
getmockIntegration(true),
distribution,
currency,
true,
sink,
true
sink
);
const result = diffLedger(newLedger, Ledger.parse(ledgerSnapshot));
expect(result).toEqual([
Expand All @@ -212,12 +209,11 @@ describe("src/core/ledger/grainIntegration", () => {
"id": "000000000000000000000A",
"type": "MARK_DISTRIBUTION_EXECUTED",
},
"ledgerTimestamp": 10,
"uuid": "000000000000000000011A",
"ledgerTimestamp": 11,
"uuid": "000000000000000000012A",
"version": "1",
},
{
ledgerTimestamp: 11,
action: {
from: "YVZhbGlkVXVpZEF0TGFzdA",
to: "000000000000000000006A",
Expand All @@ -226,10 +222,10 @@ describe("src/core/ledger/grainIntegration", () => {
type: "TRANSFER_GRAIN",
},
version: "1",
uuid: "000000000000000000012A",
"ledgerTimestamp": 12,
uuid: "000000000000000000013A",
},
{
ledgerTimestamp: 12,
action: {
from: "URgLrCxgvjHxtGJ9PgmckQ",
to: "000000000000000000006A",
Expand All @@ -238,7 +234,8 @@ describe("src/core/ledger/grainIntegration", () => {
type: "TRANSFER_GRAIN",
},
version: "1",
uuid: "000000000000000000013A",
ledgerTimestamp: 13,
uuid: "000000000000000000014A",
},
]);
});
Expand Down
8 changes: 5 additions & 3 deletions packages/sourcecred/src/core/ledger/ledger.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type AllocationReceipt = {|
*/
export type AccountingStatus = {|
+enabled: boolean,
+trackDistributions: boolean,
+currency: ?Currency,
|};
/**
Expand Down Expand Up @@ -903,7 +904,7 @@ export class Ledger {
*/
disableAccounting(): Ledger {
// ensure an external currency is set
this._getExternalCurrency();
this.externalCurrency();
// ensure accounting isn't already disabled
if (this._balanceAccountingEnabled) {
this._createAndProcessEvent({
Expand All @@ -917,7 +918,7 @@ export class Ledger {
_disableAccounting(_: DisableAccounting) {
this._balanceAccountingEnabled = false;

const currencyKey = getCurrencyKey(this._getExternalCurrency());
const currencyKey = getCurrencyKey(this.externalCurrency());

// Warning! Mutations below
const accountsIterator = this._accounts.values();
Expand Down Expand Up @@ -1000,6 +1001,7 @@ export class Ledger {
accounting(): AccountingStatus {
return {
enabled: this._balanceAccountingEnabled,
trackDistributions: this._shouldTrackGrainIntegration,
currency: this._externalCurrency,
};
}
Expand All @@ -1009,7 +1011,7 @@ export class Ledger {
* externalCurrency prop. This should only be invoked when accounting
* is disabled.
*/
_getExternalCurrency(): Currency {
externalCurrency(): Currency {
if (!this._externalCurrency) throw new Error("No External Currency Set");
return this._externalCurrency;
}
Expand Down
9 changes: 7 additions & 2 deletions packages/sourcecred/src/core/ledger/ledger.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,12 @@ const allocationId3: uuid.Uuid = uuid.random();

function getAccountingStatus(
enabled: boolean,
chainId = "1"
chainId = "1",
trackDistributions = false
): AccountingStatus {
return {
enabled,
trackDistributions,
currency: buildCurrency(chainId, ETH_CURRENCY_ADDRESS),
};
}
Expand Down Expand Up @@ -1791,7 +1793,10 @@ describe("core/ledger/ledger", () => {
parseEvmChainId("1"),
ETH_CURRENCY_ADDRESS
).removeExternalCurrency();
expect(l.accounting()).toEqual({enabled: true});
expect(l.accounting()).toEqual({
enabled: true,
trackDistributions: false,
});
});
it("cannot be unset if accounting is disabled", () => {
const l = ledgerWithActiveIdentities();
Expand Down

0 comments on commit d6021e6

Please sign in to comment.