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

Allow Grain Integrations to update their own configs #3271

Merged
merged 2 commits into from Dec 13, 2021

Conversation

topocount
Copy link
Member

@topocount topocount commented Nov 23, 2021

A grain integration might want to store some details within an instance,
such as a smart contract address. This update allows arbitrary updates
to a scoped object within the grain config file.

Merge Plan

Test Plan:

Unit tests are provided at the grain integration interface. Integration
testing will be demonstrated in the test instance using the merkle
integration.

A grain integration might want to store some details within an instance,
  such as a smart contract address. This update allows arbitrary updates
  to a scoped object within the grain config file.

Test Plan:

Unit tests are provided at the grain integration interface. Integration
testing will be demonstrated in the test instance using the merkle
integration.
Copy link
Member

@blueridger blueridger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 change requested, but otherwise approved

@@ -46,6 +49,7 @@ export type IntegrationConfig = {|
// distributed funds via a configured integration
processDistributions: boolean,
currency: Currency,
integration: ?Object,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a confusing line. Maybe it wants to be renamed config or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is confusing in the context of the named IntegrationConfig type, but I don't think its confusing once within the context of an integration. Within an integration, its pretty clearly just reading its own configuration.

From within the context of a grain integration, we have

{
  accounting,
  currency,
  processDisitributions,
  integration
}

I'd be open to renaming it self but I think naming it config would more confusing and less descriptive in the alternative context, which needs to be much more approachable for devs looking to build their own integrations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment to clarify what's going on instead.

@topocount topocount merged commit 5faa756 into accounting Dec 13, 2021
@topocount topocount deleted the config-updates branch December 13, 2021 15:40
topocount added a commit that referenced this pull request Dec 13, 2021
@topocount topocount restored the config-updates branch December 13, 2021 15:45
@topocount topocount deleted the config-updates branch December 13, 2021 15:45
topocount added a commit that referenced this pull request Dec 13, 2021
* Feature: Disable balance accounting in the ledger

When utilizing an on chain grain integration, tracking account balances
in the SourceCred ledger makes little sense. These changes disable
balance accounting, while preserving the functionality key to
SourceCred, which is tracking distributions across identities. While
balances are no longer tracked, paid totals will continue to be. This
feature, combined with Distribution execution tracking, narrows the
scope of the ledgers focus to the state of distributions, and whether or
not they have been executed outside of SourceCred.

When account balance tracking is disabled, the following changes are
made to ledger behavior:
- all grain balances are zeroed out in mutable accounts tracked by the
  ledger
- grain transfers are disabled
- distributed grain is not added to a user's balance. the paid total is
  updated, however. Undistributed balances are tracked as a whole through
  the distribution tracking functionality added earlier.
- identities without a relevant payout address will be deactivated
- identities that have no relevant payout addresses configured cannot be
  activated. relevant means the external currency configured in the
  ledger's state.

Test plan: Unit tests included

* Allow Grain Integrations to update their own configs (#3271)

A grain integration might want to store some details within an instance,
such as a smart contract address. This update allows arbitrary updates
to a scoped object within the grain config file.

Test Plan:

Unit tests are provided at the grain integration interface. Integration
testing will be demonstrated in the test instance using the merkle
integration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config return needed in Grain Integration interface
2 participants