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

Feat/425/get balances receipt #442

Merged

Conversation

@willmeister
Copy link
Collaborator

commented Sep 12, 2019

Description

Adds signatures to receipts for balances as well as roots and proofs

Metadata

Fixes

Contributing Agreement

willmeister added 12 commits Sep 9, 2019
…ate machine during async read-modify-write flows
… Also added conecpt of a RollupBlock
- Solidifying rollup block and transition concepts
- Stubbing out block submission
- Guaranteeing transitions are in order with locks

Merge branch 'master' into feat/437/PersistPendingBlock
…ex, and start/end root hash
willmeister added 2 commits Sep 13, 2019
- Removing the "Mock" label on wallet classes
- Making it so the client handles communication but passes through responses so that wallet can check signatures and handle responses as it sees fit
@ben-chain

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

Two comments:

[EDIT: ignore this one, doesn't work] 1) To add on the note above that the state receipt should include block and transition numbers--perhaps this would be reason to actually query state receipts based on the pending block. That would have the additional nice property of not having to lock the state from being updated when we query the state, since it's being pulled instead from the pending block store which will be append-only.

  1. I see that you added a distinct FaucetRequest transaction type--I'm a little confused as to whether this should be an actual transaction type, since the faucet operation is just a send. We do need to authenticate the faucet sends via the twitter API microservice, but to me that feels "out of protocol" in that it won't be reflected in anything on-chain. Ping @karlfloersch who is actually doing the contracts--thoughts?
@ben-chain

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

Oops--just realized that my suggestion in 1) doesn't work, because the pending block has roots but we don't have the ability to serve inclusion proofs from it. Never mind 😅

willmeister added 3 commits Sep 13, 2019
@willmeister

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 13, 2019

Two comments:

[EDIT: ignore this one, doesn't work] 1) To add on the note above that the state receipt should include block and transition numbers--perhaps this would be reason to actually query state receipts based on the pending block. That would have the additional nice property of not having to lock the state from being updated when we query the state, since it's being pulled instead from the pending block store which will be append-only.

  1. I see that you added a distinct FaucetRequest transaction type--I'm a little confused as to whether this should be an actual transaction type, since the faucet operation is just a send. We do need to authenticate the faucet sends via the twitter API microservice, but to me that feels "out of protocol" in that it won't be reflected in anything on-chain. Ping @karlfloersch who is actually doing the contracts--thoughts?

We might be able to make FaucetRequest just a send, but it's really multiple sends, and more importantly it's a send from an address other than the signer to the address that is the signer. I'm open to doing whatever, but do we specifically gain something by making it just a send?

@ben-chain

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2019

I agree it's not a big deal either way, but the main thing is consistency between the contract state machine and aggregator persisted state machine.

importantly it's a send from an address other than the signer to the address that is the signer

I'm not quite sure what you mean--are you saying the user receiving a faucet fund is doing the signing? I think it's just going to be the twitter hook which authenticates the faucet funding.

Copy link
Contributor

left a comment

Woot!

@willmeister willmeister merged commit 7ac4911 into plasma-group:master Sep 17, 2019
2 checks passed
2 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@willmeister willmeister deleted the willmeister:feat/425/GetBalancesReceipt branch Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.