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

payment components #110

Merged
merged 5 commits into from
Oct 31, 2024
Merged

payment components #110

merged 5 commits into from
Oct 31, 2024

Conversation

kamirr
Copy link
Contributor

@kamirr kamirr commented Oct 4, 2024

This change is Reviewable

@kamirr kamirr changed the title payment components rev1 payment components Oct 4, 2024
@kamirr kamirr requested a review from dopiera October 4, 2024 10:06
Copy link
Collaborator

@dopiera dopiera left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 17 unresolved discussions (waiting on @kamirr)


arch-snapshot/arch.md line 534 at r1 (raw file):

* Algorithm overview

### Payments

Since we're describing a payments component here, let's start with its interface. Let's not dive into code or the nitty-gritty, obscure details but the general idea. In particular I would like to learn:

  • Which other components the does payments component interacts with?
  • What is the relationship of this component to other components? (e.g. is there one per Agreement?)
  • What is the lifecycle of this component?
  • How do the other components invoke those payments (I presume there are multiple hooks in the process of performing Activities but honestly, I have no clue, so please describe them)? E.g.
    • when in the process?
    • what purpose do those serve?
    • what do they do?

If this component is further subdivided (e.g. some fixed part and swappable payment drivers), please include a decomposition of this component.

Code quote:

### Payments

arch-snapshot/arch.md line 536 at r1 (raw file):

### Payments
#### Important terms
1. Payment platform – a 3-part identifier uniquely describing a mode of

Is a "Payment platform" really only an identifier? Or is it a piece of software identified only identified by that identifier?

Code quote:

 Payment platform – a 3-part identifier 

arch-snapshot/arch.md line 537 at r1 (raw file):

#### Important terms
1. Payment platform – a 3-part identifier uniquely describing a mode of
    exchanging funds. It is composed of 3 fields:

Dumb question - why is that so? I'm guessing that the driver dictates the meaning of the network and token fields. If their meaning is implementation-defined, why do we bother to structure them into token and and network rather than have a generic, implementation defined driver_config? It seems to me that some assumptions about the driver have been made here. E.g. can we use PayU? If no - why? What are these assumptions?


arch-snapshot/arch.md line 539 at r1 (raw file):

    exchanging funds. It is composed of 3 fields:
    - driver – Determines which driver to select, see below for details.
    -  network – Defined by the driver.

extra space?


arch-snapshot/arch.md line 543 at r1 (raw file):

      The payment platform is opaque to Golem and are only relevant for payments.
      It is serialized as `{dirver}-{network}-{token}`
#### Payments models

I don't understand why there is enumeration in this subsection. Especially the second point seems disconnected from the other two. Is this the right text structure optimal here?

Code quote:

#### Payments models

arch-snapshot/arch.md line 545 at r1 (raw file):
It's unclear to me what you mean by "abstract computation". Would this be still true?

  • A payment model is an algorithm for determining the resource price based on resource usage (AKA Usage Counters)

BTW, is it the payment model, which governs whether there are mid-agreement payments? If yes, let's write explicitly about it because it is not clear neither from your or my description.

An example would be nice, so that the reader can build an intuition.

Code quote:

- A payment model is an abstract computation which describes how to map usage
  counters (see `ExeUnits` section) to an amount of currency due.

arch-snapshot/arch.md line 550 at r1 (raw file):
This point seems a bit out of place. Or was your intention to describe how an implementation of this layer is instantiated? If that is the case than please make it explicit. E.g.

A Payment Model is an inherent part of an Agreement. It is subject to negotiation between ...

Code quote:

- Negotiation:
  - Offer shall expose a property `golem.com.pricing.model` which is a string
    containing the model name, e.g. `linear`, as well as
    `golem.com.pricing.model.{model_name}` which is an object containing
    model-specific parameters.

arch-snapshot/arch.md line 555 at r1 (raw file):

  counters by constant coefficients and adds a constant on top of that, thus
  forming a linear function. The counters used today by yagna are cpu-time
  and total run-time.

This needs to be expanded. Where do the constant coefficients and the constant come from? If I'm not mistaken, the usage counters are defined by the execution layer. If so, are payment models execution unit specific?

BTW, are there no counters for GPU time or memory (be it main memory or GPU memory)?

Code quote:

- The linear payment model is the only one currently in use. It multiplies usage
  counters by constant coefficients and adds a constant on top of that, thus
  forming a linear function. The counters used today by yagna are cpu-time
  and total run-time.

arch-snapshot/arch.md line 556 at r1 (raw file):

  forming a linear function. The counters used today by yagna are cpu-time
  and total run-time.
#### Payments flow during Agreement

"during the lifetime of the Agreement", right?

Code quote:

during Agreement

arch-snapshot/arch.md line 556 at r1 (raw file):

  forming a linear function. The counters used today by yagna are cpu-time
  and total run-time.
#### Payments flow during Agreement

In general, this section looks like a collection of facts, with no guiding thought. We should change it. Please find a way to organize these facts - be it by control flow or by aspects.

Code quote:

#### Payments flow during Agreement

arch-snapshot/arch.md line 557 at r1 (raw file):

  and total run-time.
#### Payments flow during Agreement
- Payments are a crucial step of negotiations – both provider and requestor

Are they a separate step, really? I thought they are treated just like any other property

Code quote:

crucial step

arch-snapshot/arch.md line 577 at r1 (raw file):

    minted, and native tokens can be obtained from faucets, such as
    `erc20-amoy-tglm` or `erc20-holesky-tglm`.
  - As there's currently only one driver in use which requires a 1-to-1

What is a driver? This is the firs occurrence of this work in the document.

Code quote:

driver 

arch-snapshot/arch.md line 578 at r1 (raw file):

    `erc20-amoy-tglm` or `erc20-holesky-tglm`.
  - As there's currently only one driver in use which requires a 1-to-1
    relationship between `platform.token` and `platform.network`, one usually

What are these?

Code quote:

 `platform.token` and `platform.network`

arch-snapshot/arch.md line 581 at r1 (raw file):
While this is important, it doesn't seem to have anything to do with the flow, which you're describing.

BTW, please elaborate on these platforms. Please remember we agreed on the audience to be
"Technical, but let’s not assume Golem or blockchain knowledge".

Also, it's a bit misleading when you say "the Golem Network is shared by providers" - I finally understood what you meant, but I think this point would benefit from rephrasing. E.g.:

In order to make experimentation on Golem Network simple, a pool of Providers is made available to users at no cost. It has been decided that the best way of achieving it is by implementing a separate payment platform:

Code quote:

- Currently, the Golem Network is shared by providers wishing to profit, as well
  as providers for testing purposes. That is achieved by utilising two different
  kinds of payment platforms.
  - For-profit providers will declare platforms that are used for actual funds,
    e.g. `erc20-mainnet-glm` and `erc20-polygon-glm`. Those correspond to,
    respectivelly, Ethereum Mainnet and Polygon L2, on both of which no new GLM
    tokens can be minted.
  - Non-profit providers will declare platforms on which GLM tokens can be
    minted, and native tokens can be obtained from faucets, such as
    `erc20-amoy-tglm` or `erc20-holesky-tglm`.
  - As there's currently only one driver in use which requires a 1-to-1
    relationship between `platform.token` and `platform.network`, one usually
    refers to networks instead of the entire platforms. This has resulted
    in terms Mainnet(s) (referring to the first group of platforms)
    and Testnet(s) (referring to the second group).

arch-snapshot/arch.md line 590 at r1 (raw file):

    *payable*.
    - Payable DebitNotes frequency is negotiated by the property
      `golem.com.scheme.payu.debit-note.interval-sec?` and the duration between

The existence of the word scheme here makes me think there is a "payment scheme" somewhere. Am I right?

Code quote:

scheme

arch-snapshot/arch.md line 611 at r1 (raw file):

- Whenever the Requestor makes a payment and considers it done (in practice by
  doing the same confirmation the provider would), it sends a driver-defined
  blob to the Provider so that the other node can confirm the payment itself.

What does confirming payments mean? Is this the same as accepting? What happens if there is no confirmation?

Code quote:

- Whenever the Requestor makes a payment and considers it done (in practice by
  doing the same confirmation the provider would), it sends a driver-defined
  blob to the Provider so that the other node can confirm the payment itself.

arch-snapshot/arch.md line 641 at r1 (raw file):

- Verifying allocations.
- Releasing deposits.
- Reporting its status*.

I do not understand most of these capabilities. Can you please elaborate and give examples?

Code quote:

- Handling account events (locked / unlocked).
- Listing RPC endpoints*.
- Reporting account balance*.
- Reporting its name.
- Reporting the default network.
- Reporting the list of supported networks.
- Initialization.
- Reporting the need for initialization for confirming incoming payments.
- Funding – automatic obtaining funds for for Testnets.
- Transfers – transfering funds to a given address at a given platform
  w/o an allocation.
- Scheduling payments – transfering funds to a given address at a given
  platform with an allocation.
- Verifying payments.
- Verifying allocations.
- Releasing deposits.
- Reporting its status*.

@kamirr kamirr marked this pull request as ready for review October 16, 2024 12:12
@kamirr kamirr requested a review from dopiera October 16, 2024 12:12
Copy link
Contributor Author

@kamirr kamirr left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 17 unresolved discussions (waiting on @dopiera)


arch-snapshot/arch.md line 536 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

Is a "Payment platform" really only an identifier? Or is it a piece of software identified only identified by that identifier?

It is merely an identifier. The first of its components identifies the driver which is a piece of software, but I think that much has been made clear.


arch-snapshot/arch.md line 537 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

Dumb question - why is that so? I'm guessing that the driver dictates the meaning of the network and token fields. If their meaning is implementation-defined, why do we bother to structure them into token and and network rather than have a generic, implementation defined driver_config? It seems to me that some assumptions about the driver have been made here. E.g. can we use PayU? If no - why? What are these assumptions?

Network and token are the current names for these two, but they're opaque to yagna. The drivers are free to reuse those for arbitrary purposes and this wouldn't stop you from writing a PayU driver. I do think that those are bad names unless we actually want to only allow blockchain payments, but that's what it is today.


arch-snapshot/arch.md line 539 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

extra space?

Fixed


arch-snapshot/arch.md line 543 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

I don't understand why there is enumeration in this subsection. Especially the second point seems disconnected from the other two. Is this the right text structure optimal here?

Fixed


arch-snapshot/arch.md line 545 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

It's unclear to me what you mean by "abstract computation". Would this be still true?

  • A payment model is an algorithm for determining the resource price based on resource usage (AKA Usage Counters)

BTW, is it the payment model, which governs whether there are mid-agreement payments? If yes, let's write explicitly about it because it is not clear neither from your or my description.

An example would be nice, so that the reader can build an intuition.

Mid-agreement payments are not part of the payment model abstraction -- it only models pricing based on usage. Is the linear payment model not a sufficient examples?

I've reworded this because an abstract computation might in fact sound too abstract.


arch-snapshot/arch.md line 550 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

This point seems a bit out of place. Or was your intention to describe how an implementation of this layer is instantiated? If that is the case than please make it explicit. E.g.

A Payment Model is an inherent part of an Agreement. It is subject to negotiation between ...

Reworded


arch-snapshot/arch.md line 555 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

This needs to be expanded. Where do the constant coefficients and the constant come from? If I'm not mistaken, the usage counters are defined by the execution layer. If so, are payment models execution unit specific?

BTW, are there no counters for GPU time or memory (be it main memory or GPU memory)?

I've expanded this section. Regarding GPU counters -- I believe there aren't any.


arch-snapshot/arch.md line 556 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

"during the lifetime of the Agreement", right?

right


arch-snapshot/arch.md line 556 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

In general, this section looks like a collection of facts, with no guiding thought. We should change it. Please find a way to organize these facts - be it by control flow or by aspects.

Reworded


arch-snapshot/arch.md line 557 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

Are they a separate step, really? I thought they are treated just like any other property

They are cruciall semantically, you could ignore other properties but you wouldn't ignore payments. Do you think there's a better term for that?


arch-snapshot/arch.md line 577 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

What is a driver? This is the firs occurrence of this work in the document.

Done


arch-snapshot/arch.md line 578 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

What are these?

reworded


arch-snapshot/arch.md line 581 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

While this is important, it doesn't seem to have anything to do with the flow, which you're describing.

BTW, please elaborate on these platforms. Please remember we agreed on the audience to be
"Technical, but let’s not assume Golem or blockchain knowledge".

Also, it's a bit misleading when you say "the Golem Network is shared by providers" - I finally understood what you meant, but I think this point would benefit from rephrasing. E.g.:

In order to make experimentation on Golem Network simple, a pool of Providers is made available to users at no cost. It has been decided that the best way of achieving it is by implementing a separate payment platform:

Reworded. I believe this is a good section for this because testnets/mainnets tie closely with negotiation and not much else, really. It could be somewhere else, but then you'd have to jump around the document for no clear reason.

As for blockchain-technicalities -- the concepts themselves are fundamental to the operation of the network as it is, I don't think we can ommit them. Providing all context for somebody with no familiarity with ethereum is also not feasible. While I understand the decision to not assume blockchain knowledge in general, it is unavoidable for describing payments in any level of detail. All in all, I strongly believe this section should retain it's merit and assume that the reader understands Ethereum to some degree (do note that those aren't advanced concepts at all, but rather very common solutions).


arch-snapshot/arch.md line 590 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

The existence of the word scheme here makes me think there is a "payment scheme" somewhere. Am I right?

It's scattered across GAPs (https://github.com/golemfactory/golem-architecture/tree/master/gaps) and I'm pretty sure it's not nearly complete. If there is a scheme somewhere, it's the code.


arch-snapshot/arch.md line 611 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

What does confirming payments mean? Is this the same as accepting? What happens if there is no confirmation?

Confirmation is confirmation -- checking whether a payment has actually been made successfully. It's the same as with normal banking -- you may be asked about a Transfer Confirmation, which would be a pdf document produced by the bank.

In case of blockchain it's a bit more convoluted because you only get a transaction ID from the requestor, and the provider must consult the blockchain itself (being the central authority) to see whether the transaction with the given ID actually exists and it's parameters match what the Requestor declared and the Provider expected.

Confirmations are also a fundamental term in Ethereum (and most other blockchains), which I suppose ties back to the assumption that we can produce a technical document regarding blockchain while assuming no knowledge of blockchain. See one of my previous comments for my thoughts on that.


arch-snapshot/arch.md line 641 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

I do not understand most of these capabilities. Can you please elaborate and give examples?

Done

Copy link
Collaborator

@dopiera dopiera left a comment

Choose a reason for hiding this comment

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

I think it looks much better now, thank you. I do have some more comments, though.

PTAL at the rest of the document - we're attempting to:

  • call entities in the system consistently (e.g. create a distinction between a Requestor (person) and a Requestor Agent (piece of software) - please take a look at the code already merged and try to mimic this
  • cross reference the terms used (i.e. put links to relevant sections to make it easy for the reader to navigate) - please add such links to your text - also use the already merged documentation for reference
  • capitalize the terms which have special meaning in Golem (e.g. Requestor) - please make sure you do, too

I think we should describe batching, too.

Reviewable status: 0 of 1 files reviewed, 15 unresolved discussions (waiting on @kamirr)


arch-snapshot/arch.md line 557 at r1 (raw file):
ACK. Maybe "element" then? Or "aspect"? Or rewrite the sentence altogether to something along these lines:

Provider and Requestor must establish the mechanics of payments (Payment Driver, its parameters (e.g. specific blockchain), Payment Model) during the agreement Negotiation.


arch-snapshot/arch.md line 611 at r1 (raw file):

Previously, kamirr (Kamil Koczurek) wrote…

Confirmation is confirmation -- checking whether a payment has actually been made successfully. It's the same as with normal banking -- you may be asked about a Transfer Confirmation, which would be a pdf document produced by the bank.

In case of blockchain it's a bit more convoluted because you only get a transaction ID from the requestor, and the provider must consult the blockchain itself (being the central authority) to see whether the transaction with the given ID actually exists and it's parameters match what the Requestor declared and the Provider expected.

Confirmations are also a fundamental term in Ethereum (and most other blockchains), which I suppose ties back to the assumption that we can produce a technical document regarding blockchain while assuming no knowledge of blockchain. See one of my previous comments for my thoughts on that.

Why not include what you just wrote in the doc? I'm pretty sure you can come up with a two-sentence explanation of what how the confirmations relate to blockchain concepts and what purpose they serve.


arch-snapshot/arch.md line 544 at r2 (raw file):

  - token – Defined by the driver.

  The payment platform is opaque to Golem and are only relevant for payments.

is?

Code quote:

are

arch-snapshot/arch.md line 547 at r2 (raw file):

  It is serialized as `{dirver}-{network}-{token}`.
#### Payments models
A payment model is an algorithm for determining the amount due based on resource usage (AKA Usage Counters, see `ExeUnits` section).

Link please

Code quote:

ExeUnits

arch-snapshot/arch.md line 552 at r2 (raw file):

counters by constant coefficients and adds a constant on top of that, thus
forming a linear function. The counters used today by yagna are cpu-time
and total run-time.

Are you absolutely sure of that? Is this the case also for the GamerHash or the GPU provider? @nieznanysprawiciel - can you confirm this?

Code quote:

forming a linear function. The counters used today by yagna are cpu-time
and total run-time.

arch-snapshot/arch.md line 555 at r2 (raw file):

The model parameters (for example coefficients of the linear Payment Model) are
declared by the provider. Based on those, the requestor can make an informed

In an Offer, right? If that is the case, then please link make it explicit.

Code quote:

The model parameters (for example coefficients of the linear Payment Model) are
declared by the provider. Based on those, the requestor can make an informed

arch-snapshot/arch.md line 569 at r2 (raw file):

    containing the model name, e.g. `linear`, as well as
    `golem.com.pricing.model.{model_name}` which is an object containing
    model-specific parameters.

That's interesting. According to #108 there are is no "object" type in the property language. It looks like we should chat over video to determine which is true.

Code quote:

    `golem.com.pricing.model.{model_name}` which is an object containing
    model-specific parameters.

arch-snapshot/arch.md line 654 at r2 (raw file):

  - Accessing Ethereum is done via servers commonly called RPC endpoints.
    A faulty endpoint could cause issues with transaction processing, so the
    driver exposes them with some metadata to allow checking whether they work. 

It might be a dumb question, but I'll ask it anyway.

My understanding is that you're talking about Ethereum nodes' endpoints. What if there was a payment driver which didn't use the RPC API but had its own Ethereum node implementation linked in? Or didn't use Ethereum at all?

Code quote:

- Listing RPC endpoints*.
  - Accessing Ethereum is done via servers commonly called RPC endpoints.
    A faulty endpoint could cause issues with transaction processing, so the
    driver exposes them with some metadata to allow checking whether they work.

arch-snapshot/arch.md line 668 at r2 (raw file):

- Initialization.
  - Drivers may need to perform some specific to them work before being able
    to work. This method is the called before any transactions are scheduled.

This point is grammatically weird - can you please let chatgpt do its magic?

Code quote:

  - Drivers may need to perform some specific to them work before being able
    to work. This method is the called before any transactions are scheduled.

arch-snapshot/arch.md line 675 at r2 (raw file):

- Funding – automatic obtaining funds for for Testnets.
  - Some Testnets allow obtaining funds (both native token and GLM)
  - programmatically. This method does this if possible.

There's a weird line break here.

Code quote:

  - Some Testnets allow obtaining funds (both native token and GLM)
  - programmatically. This method does this if possible.

arch-snapshot/arch.md line 677 at r2 (raw file):

  - programmatically. This method does this if possible.
- Transfers – transfering funds to a given address at a given platform
  w/o an allocation.

What are allocations? Please add a description in the "Important terms" section.

Code quote:

allocation

arch-snapshot/arch.md line 682 at r2 (raw file):

- Confirming payments.
  - A driver defines a heuristic for deciding whether a payment has been done
    correctly and is to be trusted.

I still don't get it. Why do we need a heuristic rather than just a simple check?

Code quote:

- Confirming payments.
  - A driver defines a heuristic for deciding whether a payment has been done
    correctly and is to be trusted.

arch-snapshot/arch.md line 703 at r2 (raw file):
We don't need to describe it, so please change it to

now obsolete, another payment driver build on Ethereum

Code quote:

 [TODO] zksync driver predates my experience with web3.

Copy link
Contributor Author

@kamirr kamirr left a comment

Choose a reason for hiding this comment

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

If the user configures specific constraints on the demand, who's choosing the provider -- Requestor or Requestor Agent?

Re. batching, done.

Reviewable status: 0 of 1 files reviewed, 15 unresolved discussions (waiting on @dopiera and @nieznanysprawiciel)


arch-snapshot/arch.md line 557 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

ACK. Maybe "element" then? Or "aspect"? Or rewrite the sentence altogether to something along these lines:

Provider and Requestor must establish the mechanics of payments (Payment Driver, its parameters (e.g. specific blockchain), Payment Model) during the agreement Negotiation.

Done


arch-snapshot/arch.md line 611 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

Why not include what you just wrote in the doc? I'm pretty sure you can come up with a two-sentence explanation of what how the confirmations relate to blockchain concepts and what purpose they serve.

Done


arch-snapshot/arch.md line 544 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

is?

Done


arch-snapshot/arch.md line 547 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

Link please

Done


arch-snapshot/arch.md line 552 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

Are you absolutely sure of that? Is this the case also for the GamerHash or the GPU provider? @nieznanysprawiciel - can you confirm this?

I am. This just means that GPU Providers can't offer usage-based pricing, but they can obviously raise the price to match the hardware offered. Note that unlike CPU, you cannot spin up multiple providers utilising one GPU (it's passed through to the VM directly and exclusively), meaning the GPU is wholly rented to the given requestor. This makes usage-based pricing less useful and I guess that's why it's not implemented.


arch-snapshot/arch.md line 555 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

In an Offer, right? If that is the case, then please link make it explicit.

clarified


arch-snapshot/arch.md line 569 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

That's interesting. According to #108 there are is no "object" type in the property language. It looks like we should chat over video to determine which is true.

Object as in givent golem.com.pricing.model.linear there exist properties of the form golem.com.pricing.model.linear.PROP-NAME. I don't think there's a better name for such a construct than object, but I can elaborate, although I believe this should be defined somewhere in the market offer specification language section.


arch-snapshot/arch.md line 654 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

It might be a dumb question, but I'll ask it anyway.

My understanding is that you're talking about Ethereum nodes' endpoints. What if there was a payment driver which didn't use the RPC API but had its own Ethereum node implementation linked in? Or didn't use Ethereum at all?

This is the problem with the current design. Notice that this point is marked with a *, I comment on that fact right below the list.


arch-snapshot/arch.md line 668 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

This point is grammatically weird - can you please let chatgpt do its magic?

Done, although I'm not all that sure it's any better.


arch-snapshot/arch.md line 675 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

There's a weird line break here.

Fixed


arch-snapshot/arch.md line 677 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

What are allocations? Please add a description in the "Important terms" section.

Done


arch-snapshot/arch.md line 682 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

I still don't get it. Why do we need a heuristic rather than just a simple check?

This is due to the nature of ethereum -- it can temporarily split into inconsistent forks. The more blocks are confirmed since the transaction landed in the blokchain, the more confident can you be about it.

Note that calling this a heuristic is somewhat pedantic of me (it can't realistically fail), but that's how it is. I've elaborated on it according to your request in another comment, this should make it clear to the reader.


arch-snapshot/arch.md line 703 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

We don't need to describe it, so please change it to

now obsolete, another payment driver build on Ethereum

Done

@kamirr kamirr requested a review from dopiera October 23, 2024 09:13
Copy link
Collaborator

@dopiera dopiera left a comment

Choose a reason for hiding this comment

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

I get your point - for this case pick the one which your gut tells you is more frequent :)

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @kamirr)


arch-snapshot/arch.md line 703 at r2 (raw file):

Previously, kamirr (Kamil Koczurek) wrote…

Done

nit: s/build/built/

Copy link
Contributor Author

@kamirr kamirr left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @dopiera)


arch-snapshot/arch.md line 534 at r1 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

Since we're describing a payments component here, let's start with its interface. Let's not dive into code or the nitty-gritty, obscure details but the general idea. In particular I would like to learn:

  • Which other components the does payments component interacts with?
  • What is the relationship of this component to other components? (e.g. is there one per Agreement?)
  • What is the lifecycle of this component?
  • How do the other components invoke those payments (I presume there are multiple hooks in the process of performing Activities but honestly, I have no clue, so please describe them)? E.g.
    • when in the process?
    • what purpose do those serve?
    • what do they do?

If this component is further subdivided (e.g. some fixed part and swappable payment drivers), please include a decomposition of this component.

Please check if the new revision answers your questions


arch-snapshot/arch.md line 703 at r2 (raw file):

Previously, dopiera (Marek Dopiera) wrote…

nit: s/build/built/

Done

@kamirr kamirr requested a review from dopiera October 31, 2024 10:06
@kamirr kamirr self-assigned this Oct 31, 2024
Copy link
Collaborator

@dopiera dopiera left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @kamirr)

@kamirr kamirr merged commit 37445c4 into new-arch Oct 31, 2024
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.

2 participants