Skip to content

Conversation

cy-utkarshM
Copy link
Collaborator

  • Added base features
  • Refactored firmware layout
  • Backed up intermediate working states

@muzaffarbhat07
Copy link
Collaborator

resolve conflicts and fix ci

@@ -186,9 +190,10 @@ static bool validate_request_data(btc_get_public_key_request_t *request) {
memcpy(data.params,
request->initiate.wallet_id,
sizeof(request->initiate.wallet_id));
#ifndef BTC_ONLY_BUILD
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move the above defined variables(for exchange app function call) also inside the if block

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@@ -302,7 +307,9 @@ void btc_get_pub_key(btc_query_t *query) {
memzero(seed, sizeof(seed));

if (sign_address) {
#ifndef BTC_ONLY_BUILD
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep the whole if (sign_address) block inside the #ifndef BTC_ONLY_BUILD block. Also, keep the sign_address variable declaration statement in the #ifndef BTC_ONLY_BUILD

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@@ -588,10 +591,12 @@ static bool get_user_verification() {
}

if (use_signature_verification) {
#ifndef BTC_ONLY_BUILD
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move all code related to exchange app inside the #ifndef BTC_ONLY_BUILD block. Be it variable(global or local) declarations or if conditions or function calls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed static bool use_signature_verification = false; from the global/static variables section.

Added bool use_signature_verification = false; inside the #ifndef BTC_ONLY_BUILD block within validate_request_data. This makes use_signature_verification a local variable to validate_request_data when BTC_ONLY_BUILD is not defined.

Added bool use_signature_verification = false; inside the #ifndef BTC_ONLY_BUILD block within get_user_verification. This makes use_signature_verification a local variable to get_user_verification when BTC_ONLY_BUILD is not defined. I also kept your comment explaining the current state of this variable's usage.

Copy link
Collaborator

@muzaffarbhat07 muzaffarbhat07 Jun 25, 2025

Choose a reason for hiding this comment

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

Why is static bool use_signature_verification = false; removed from the global/static variables section and added as local variable? This breaks exchange flow.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've pushed a fix that corrects this. use_signature_verification is now a static variable again at the file level to preserve its state across function calls, but the entire declaration is properly wrapped within the #ifndef BTC_ONLY_BUILD block.

// free(meta_data_arr[i].data_struct.coin_data);
// }
// }
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this code commented?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this code was never used, after talking to Pulkit I removed it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, but we can also just guard it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay then I will guard it then!

registry_add_app(get_ltc_app_desc());
registry_add_app(get_doge_app_desc());
registry_add_app(get_dash_app_desc());
registry_add_app(get_inheritance_app_desc());
Copy link
Collaborator

Choose a reason for hiding this comment

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

inheritance app is added twice

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@cy-utkarshM cy-utkarshM force-pushed the feat/test_btc branch 3 times, most recently from a505414 to a2fe260 Compare June 25, 2025 07:39
@cy-utkarshM cy-utkarshM force-pushed the feat/test_btc branch 2 times, most recently from 47379d7 to ad7911c Compare June 27, 2025 05:57
@@ -49,4 +51,4 @@ jobs:
run: |
content_type=$(file -b --mime-type Main-Release-outputs/Cypherock-Main.bin)
curl -X POST -H "Content-Type: ${content_type}" -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${auth_token}" -H "X-GitHub-Api-Version: 2022-11-28" ${upload_url}?name=Cypherock-Main.bin --data-binary @Main-Release-outputs/Cypherock-Main.bin
curl -X POST -H "Content-Type: ${content_type}" -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${auth_token}" -H "X-GitHub-Api-Version: 2022-11-28" ${upload_url}?name=version.txt --data-binary @version.txt
curl -X POST -H "Content-Type: ${content_type}" -H "Accept: application/vnd.github+json" -H "Authorization: Bearer ${auth_token}" -H "X-GitHub-Api-Version: 2022-11-28" ${upload_url}?name=version.txt --data-binary @version.txt
Copy link
Collaborator

@muzaffarbhat07 muzaffarbhat07 Jun 27, 2025

Choose a reason for hiding this comment

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

Upload btc only firmware to github also. Also generate its hash. Take reference from odix pr

CMakeLists.txt Outdated

OPTION(DEV_SWITCH "Additional features/logs to aid developers" OFF)
OPTION(UNIT_TESTS_SWITCH "Compile build for main firmware or unit tests" OFF)
OPTION(BTC_ONLY "Build firmware for Bitcoin only" OFF)

Copy link
Collaborator

@muzaffarbhat07 muzaffarbhat07 Jun 27, 2025

Choose a reason for hiding this comment

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

In case of btc only, append -btc to project name. Take reference from odix pr

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

"${PROTO_COMMON_SRC}/btc"
"${PROTO_COMMON_SRC}/inheritance"
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Top level common proto files(like core.proto, error.proto) are missing in btc only

Copy link
Collaborator

@muzaffarbhat07 muzaffarbhat07 left a comment

Choose a reason for hiding this comment

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

Can you explain the need to change the variable name EXECUTABLE to PROJECT

@cy-utkarshM
Copy link
Collaborator Author

Can you explain the need to change the variable name EXECUTABLE to PROJECT

The PROJECT_NAME variable already comes from the top-level CMakeLists.txt, and it is guaranteed to be the correct project identifier (e.g., Cypherock-Main-btc).
Creating a new variable (EXECUTABLE) that just wraps ${PROJECT_NAME}.elf was redundant.

@cy-utkarshM cy-utkarshM changed the title Initial Bitcoin-only firmware integration: Bitcoin-only firmware integration Aug 17, 2025
@muzaffarbhat07 muzaffarbhat07 changed the base branch from develop to feat/btc-only/base September 3, 2025 15:17
This reverts commit 4b242d2.
This reverts commit b92ed97.
@muzaffarbhat07 muzaffarbhat07 merged commit c7e6786 into feat/btc-only/base Sep 4, 2025
6 checks passed
@muzaffarbhat07 muzaffarbhat07 deleted the feat/test_btc branch September 4, 2025 09:02
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