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

can now claim gift codes to fog enabled addresses #212

Merged
merged 4 commits into from
Dec 14, 2021

Conversation

briancorbin
Copy link
Contributor

No description provided.

full-service/src/service/gift_code.rs Outdated Show resolved Hide resolved
Comment on lines 569 to 576
let fog_resolver = {
let fog_uris = core::slice::from_ref(&recipient_public_address)
.iter()
.filter_map(|x| extract_fog_uri(x).transpose())
.collect::<Result<Vec<_>, _>>()?;
(self.fog_resolver_factory)(&fog_uris.as_slice())
.map_err(GiftCodeServiceError::UnexpectedTxStatus)?
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be simplified to something like this since there is only a single address here:

let fog_resolver = {
    // This should convert the Option<String> into Option<Result<FogUri, Err>> and then transpose will convert to Result<Option<FogUri>, Err>. The ? operator will then return the error if there is one, and if not fog_uri will be Option<FogUri>
    let fog_uri = recipient_public_address.fog_report_url().map(FogUri::from_str).transpose().map_err(GiftCodeServiceError::InvaligFogUri)?;

    let uri_slice = if let Some(uri) = fog_uri {
        &[uri]
    } else {
        &[]
    };

    (self.fog_resolver_factory)(uri_slice).map_err(GiftCodeServiceError::FogPubkeyResolver)?;
}

You'll need to add error types to GiftCodeServiceError. UnexpectedTxStatus is not suitable for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually maybe this isn't any better, but the comment on the error variant is still relevant.
I believe core::slice::from_ref(&x) can be replaced with &[&x].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, thanks for catching that, meant to change that after getting it working!

Copy link
Contributor

@itdaniher itdaniher left a comment

Choose a reason for hiding this comment

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

YAY BRIAN!

@briancorbin briancorbin requested a review from eranrund December 13, 2021 22:08
@briancorbin briancorbin merged commit bf5b4df into develop Dec 14, 2021
@briancorbin briancorbin deleted the bugfix/gift_claim_fog branch December 14, 2021 00:03
briancorbin added a commit that referenced this pull request Jan 18, 2022
* removed US IP block (#181)

* Cold wallet changes (#190)

* Improvements to MobCLI messages and documentation.
* Remove external dependencies for command line client.
* Clean up default full-service executable names in command line env vars. Add env var script and install script to setup.py.
* Source the mobilecoin environment file after installing.
* Work around a bug in the full-service status endpoint which misreports local_block_index.
* Remove delay feature, which was reporting the tombstone block incorrectly.
* Add --fee argument to mobcli send.
* Put utility functions for address and receipt manipulation in their own module.

* Add generating of receiver receipts to CLI. Fix an off-by-one error checking tombstone block validity. (#177)

* Minor improvements to CLI messages and documentation. (#186)

* Feature/add tx proposal to build and submit (#192)

* added option to log a transaction when building (#194)

* Add an option to `mobcli export` to show entropy onscreen. (#195)

* Optionally return tx_proposal with build_and_submit_transaction (#198)

* Optionally return tx_proposal with build_and_submit_transaction

* Add assertion for build_and_submit_transaction when returning a tx_proposal

* Encapsulate original build_and_submit_request logic, add two new methods instead to avoid breaking change

* Remove params for build_and_submit_transaction_with_proposal from integration tests

* Key name for tx_proposal in response changed

* Debug CLI

* Remove debug code

* bugfix for tx selection:  (#206)

* bugfix for tx selection: prior to this change full-service would select the smallest TXOs, even though all previous checks look at the large ones

* adding test

* resetting to sort ascending and removing reversal when determining val

* fixing flaky tests

* updating tests

* updating tests and fixing some flaky stuff

* fixing flaky test

* fixing a few more flaky tests

* fixing flaky test

* fixing bug (finally)

* breaking up large test into multiple descriptive tests

Co-authored-by: Brian <brian.william.corbin@gmail.com>

* No need to use segno fork anymore. (#204)

* fix mac os build step (#209)

* Tests and fixes (#207)

* Updating documentation

* Adding change verification after scan to log_submitted test

* Add test for big_int and self-send for transaction_log db

* Clean up usage of block-index vs block-height (#196)

* Add an option to `mobcli export` to show entropy onscreen.

* Clean up block-index vs block-height.

A block index is the index number of a specific block. A block height is
a count of a range of blocks. This was not consistent across the API and
codebase.

Change the API for account and ledger syncing to use block heights. This
also fixes some bugs in sync reporting during offline mode.

Update the command line client to handle the new API.

Update the docs in API.md

* GitBook: [feature/block-height] 4 pages modified

* Remove legacy docs for database usage, now that gitbook has them. Clean up gitbook errors.

* Remove legacy API.md. Remove spurious files created by gitbook.

* Update usage of block_index in gitbook docs.

* Rust formatting.

Co-authored-by: Brian <brian.william.corbin@gmail.com>

* add cli arg to enable fog (#199)

* Add exit codes to indicate full-service process exit reason. (#193)

* Add exit codes to indicate full-service process exit reason.

* Rust formatting.

* minimum viable change to jsonrpc's ID field - move from i32 to serde_json::Value (#203)

* Get Subaddress by Index (#210)

* Clean up usage of ingest enclave setting in CLI. (#211)

* can now claim gift codes to fog enabled addresses (#212)

* can now claim gift codes to fog enabled addresses

* Update full-service/src/service/gift_code.rs

Fix documentation typo.

Co-authored-by: Eran Rundstein <eran@rundste.in>

* removed unnecessary functions and cleaned up code and added correct err

Co-authored-by: itdaniher <itdaniher@gmail.com>
Co-authored-by: Eran Rundstein <eran@rundste.in>

* Fix an off-by-one error calculating account_block_height for balances. (#214)

* Improve help when incorrectly typing "mobcli address". (#213)

* We should be showing the change subaddress in mobcli address list. (#216)

* Feature/refactor txo status (#215)

* WIP removing account_txo_statuses.

* Create migration to add minted and received account id to txo table.

* create_received, create_minted, update_to_pending

* select_by_public_key

* select_by_id

* list_for_account

* break up list_txos by type and status

* WIP reenabling tests

* txo service tests

* transaction builder tests

* receipt tests

* WIP tests

* WIP

* WIP

* WIP - list minted

* cleaning up unused imports

* removing txodetails fully

* test get all txos

* test_remove_account_from_txo

* fix are_all_spent

* fix delete unreferenced txos

* fixing clippy issues

* reproducing a known bug

check receipt status is returning a transaction pending result when it should in fact return a failure to decrypt value for a receipt with a txo that is unowned by the checker

* reimplementing and rewriting existing tests for transaction logs

* removing account txo status

* cargo fmt

* determining correct status

* WIP data migrations.

* Fix a bug where spent txos are being selected for building a transaction.

* Add data migrations.

* Fix a query which incorrectly identified secreted txos for an account.

* fixing comment issue

* updating comment for receipt.rs bug

* Clarify why get_balance_for_address cannot report orphaned balance.

* refactored unwrapping and throwing error in receipt.rs

Co-authored-by: Christian Oudard <christian.oudard@gmail.com>

* Feature/uprev mobilecoin library (#219)

* upreving mobilecoin library to current master

* impl error handing for mc_util_serial::DecodeError

* fixed TransactionBuilder memo field, removed unused crates

* fixing test utils

* remove mc-attest-core dep

* update CI docker image

* dep updates and cleanups

* clippy fixes

* whitespace

Co-authored-by: Eran Rundstein <eran@rundste.in>

* Excluding mobilecoin submodule from linter (#220)

* Show orphaned txos in CLI account list. (#227)

* Implement fees correctly in mobcli. (#228)

* Davey/update docs (#225)

* update non-txo docs

* comb remaining documentation

* revert submodule hash change

* clean up docs summary

* revise docs structure, update README for stats

Co-authored-by: Davey <davey@mobilecoin.com>

* create release scripts

* fixed some txos not being marked as unspent/pending correctly

* cargo fmt fixes

* fixing release script

Co-authored-by: Christian Oudard <christian.oudard@gmail.com>
Co-authored-by: Greg Rice <gregrice@gmail.com>
Co-authored-by: Eran Rundstein <eran@rundste.in>
Co-authored-by: sugargoat <sara.drakeley@mobilecoin.com>
Co-authored-by: Jason Greathouse <jgreat@mobilecoin.com>
Co-authored-by: itdaniher <itdaniher@gmail.com>
Co-authored-by: wjuan-mob <william@mobilecoin.com>
Co-authored-by: Davey Alvarez <daveyalvarez03@gmail.com>
Co-authored-by: Davey <davey@mobilecoin.com>
briancorbin added a commit that referenced this pull request Jan 25, 2022
* removed US IP block (#181)

* Cold wallet changes (#190)

* Improvements to MobCLI messages and documentation.
* Remove external dependencies for command line client.
* Clean up default full-service executable names in command line env vars. Add env var script and install script to setup.py.
* Source the mobilecoin environment file after installing.
* Work around a bug in the full-service status endpoint which misreports local_block_index.
* Remove delay feature, which was reporting the tombstone block incorrectly.
* Add --fee argument to mobcli send.
* Put utility functions for address and receipt manipulation in their own module.

* Add generating of receiver receipts to CLI. Fix an off-by-one error checking tombstone block validity. (#177)

* Minor improvements to CLI messages and documentation. (#186)

* Feature/add tx proposal to build and submit (#192)

* added option to log a transaction when building (#194)

* Add an option to `mobcli export` to show entropy onscreen. (#195)

* Optionally return tx_proposal with build_and_submit_transaction (#198)

* Optionally return tx_proposal with build_and_submit_transaction

* Add assertion for build_and_submit_transaction when returning a tx_proposal

* Encapsulate original build_and_submit_request logic, add two new methods instead to avoid breaking change

* Remove params for build_and_submit_transaction_with_proposal from integration tests

* Key name for tx_proposal in response changed

* Debug CLI

* Remove debug code

* bugfix for tx selection:  (#206)

* bugfix for tx selection: prior to this change full-service would select the smallest TXOs, even though all previous checks look at the large ones

* adding test

* resetting to sort ascending and removing reversal when determining val

* fixing flaky tests

* updating tests

* updating tests and fixing some flaky stuff

* fixing flaky test

* fixing a few more flaky tests

* fixing flaky test

* fixing bug (finally)

* breaking up large test into multiple descriptive tests

Co-authored-by: Brian <brian.william.corbin@gmail.com>

* No need to use segno fork anymore. (#204)

* fix mac os build step (#209)

* Tests and fixes (#207)

* Updating documentation

* Adding change verification after scan to log_submitted test

* Add test for big_int and self-send for transaction_log db

* Clean up usage of block-index vs block-height (#196)

* Add an option to `mobcli export` to show entropy onscreen.

* Clean up block-index vs block-height.

A block index is the index number of a specific block. A block height is
a count of a range of blocks. This was not consistent across the API and
codebase.

Change the API for account and ledger syncing to use block heights. This
also fixes some bugs in sync reporting during offline mode.

Update the command line client to handle the new API.

Update the docs in API.md

* GitBook: [feature/block-height] 4 pages modified

* Remove legacy docs for database usage, now that gitbook has them. Clean up gitbook errors.

* Remove legacy API.md. Remove spurious files created by gitbook.

* Update usage of block_index in gitbook docs.

* Rust formatting.

Co-authored-by: Brian <brian.william.corbin@gmail.com>

* add cli arg to enable fog (#199)

* Add exit codes to indicate full-service process exit reason. (#193)

* Add exit codes to indicate full-service process exit reason.

* Rust formatting.

* minimum viable change to jsonrpc's ID field - move from i32 to serde_json::Value (#203)

* Get Subaddress by Index (#210)

* Clean up usage of ingest enclave setting in CLI. (#211)

* can now claim gift codes to fog enabled addresses (#212)

* can now claim gift codes to fog enabled addresses

* Update full-service/src/service/gift_code.rs

Fix documentation typo.

Co-authored-by: Eran Rundstein <eran@rundste.in>

* removed unnecessary functions and cleaned up code and added correct err

Co-authored-by: itdaniher <itdaniher@gmail.com>
Co-authored-by: Eran Rundstein <eran@rundste.in>

* Fix an off-by-one error calculating account_block_height for balances. (#214)

* Improve help when incorrectly typing "mobcli address". (#213)

* We should be showing the change subaddress in mobcli address list. (#216)

* Feature/refactor txo status (#215)

* WIP removing account_txo_statuses.

* Create migration to add minted and received account id to txo table.

* create_received, create_minted, update_to_pending

* select_by_public_key

* select_by_id

* list_for_account

* break up list_txos by type and status

* WIP reenabling tests

* txo service tests

* transaction builder tests

* receipt tests

* WIP tests

* WIP

* WIP

* WIP - list minted

* cleaning up unused imports

* removing txodetails fully

* test get all txos

* test_remove_account_from_txo

* fix are_all_spent

* fix delete unreferenced txos

* fixing clippy issues

* reproducing a known bug

check receipt status is returning a transaction pending result when it should in fact return a failure to decrypt value for a receipt with a txo that is unowned by the checker

* reimplementing and rewriting existing tests for transaction logs

* removing account txo status

* cargo fmt

* determining correct status

* WIP data migrations.

* Fix a bug where spent txos are being selected for building a transaction.

* Add data migrations.

* Fix a query which incorrectly identified secreted txos for an account.

* fixing comment issue

* updating comment for receipt.rs bug

* Clarify why get_balance_for_address cannot report orphaned balance.

* refactored unwrapping and throwing error in receipt.rs

Co-authored-by: Christian Oudard <christian.oudard@gmail.com>

* Feature/uprev mobilecoin library (#219)

* upreving mobilecoin library to current master

* impl error handing for mc_util_serial::DecodeError

* fixed TransactionBuilder memo field, removed unused crates

* fixing test utils

* remove mc-attest-core dep

* update CI docker image

* dep updates and cleanups

* clippy fixes

* whitespace

Co-authored-by: Eran Rundstein <eran@rundste.in>

* Excluding mobilecoin submodule from linter (#220)

* Show orphaned txos in CLI account list. (#227)

* Implement fees correctly in mobcli. (#228)

* Davey/update docs (#225)

* update non-txo docs

* comb remaining documentation

* revert submodule hash change

* clean up docs summary

* revise docs structure, update README for stats

Co-authored-by: Davey <davey@mobilecoin.com>

* create release scripts

* fixed some txos not being marked as unspent/pending correctly

* cargo fmt fixes

* fixing release script

* Ledger Validator Service (#235)

* Skeleton for the Ledger Validator Node (#221)

* Stub for validator service

* remove unused

* Removing api from validator service, using existing api

* Cargo fmt

* Removing unused dependencies in cargo toml

* Undo overzealous removal

* wip

* make some full-service configuration re-usable, fix compile errors

* fmt

Co-authored-by: wjuan-mob <william@mobilecoin.com>

* Basic Validator GRPC API (#222)

* move validator-service to validator/service

* update copyright

* basic api crate

* use validator uri in config

* basic grpc service

* basic proto api

* noop impls

* add services

* LedgerDb syncing (#223)

* make ledger db structopt stuff reusable

* plumb in ledger syncing

* fmt

* lint

* Initial take on API endpoint implementation (#224)

* pass LedgerDb to service

* fix rpc logger usage

* get_archive_blocks_impl

* propose_tx_impl

* fetch_fog_report_impl

* get_last_block_info_impl

* Support using the validator service as a backend for full-service (#226)

* configuration support for validator mode, basic fork scheme

* initial validator client impl

* ledger bootstrapping via validator

* ledger syncing

* connection trait impls

* more plumbing and rocket wrestling, sending transaction worked

* fog plumbing

* update comment

* lint

* Add README (#229)

* add readme

* fix quotes

* Update validator/README.md

Co-authored-by: Chris Beck <beck.ct@gmail.com>

* Update validator/README.md

Co-authored-by: Chris Beck <beck.ct@gmail.com>

* readme fixes

Co-authored-by: Chris Beck <beck.ct@gmail.com>

Co-authored-by: wjuan-mob <william@mobilecoin.com>
Co-authored-by: Chris Beck <beck.ct@gmail.com>

* First draft removing ip check feature and adding ipcheck for validator (#237)

* First draft removing ip check feature and adding ipcheck for validator

* Addressing code review comment

* Adding comments

* updated cargo.lock

Co-authored-by: Christian Oudard <christian.oudard@gmail.com>
Co-authored-by: Greg Rice <gregrice@gmail.com>
Co-authored-by: Eran Rundstein <eran@rundste.in>
Co-authored-by: sugargoat <sara.drakeley@mobilecoin.com>
Co-authored-by: Jason Greathouse <jgreat@mobilecoin.com>
Co-authored-by: itdaniher <itdaniher@gmail.com>
Co-authored-by: wjuan-mob <william@mobilecoin.com>
Co-authored-by: Davey Alvarez <daveyalvarez03@gmail.com>
Co-authored-by: Davey <davey@mobilecoin.com>
Co-authored-by: Chris Beck <beck.ct@gmail.com>
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.

3 participants