add unix specific execution permission to tp binary executable #1293
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1293 +/- ##
==========================================
- Coverage 19.30% 17.39% -1.91%
==========================================
Files 164 163 -1
Lines 10849 10675 -174
==========================================
- Hits 2094 1857 -237
- Misses 8755 8818 +63
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Bencher Report
|
| Branch | 2024-12-12-fix-template-provider-binary-permission |
| Testbed | sv1 |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Benchmark Result estimated cycles (Result Δ%) | Upper Boundary estimated cycles (Limit %) | Instructions | Benchmark Result instructions (Result Δ%) | Upper Boundary instructions (Limit %) | L1 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | L2 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | RAM Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| get_authorize | 📈 view plot 🚷 view threshold | 8,384.00 (-0.87%) | 8,724.79 (96.09%) | 📈 view plot 🚷 view threshold | 3,663.00 (-2.05%) | 3,869.70 (94.66%) | 📈 view plot 🚷 view threshold | 5,109.00 (-2.51%) | 5,451.74 (93.71%) | 📈 view plot 🚷 view threshold | 11.00 (+45.01%) | 14.75 (74.59%) | 📈 view plot 🚷 view threshold | 92.00 (+1.29%) | 95.76 (96.08%) |
| get_submit | 📈 view plot 🚷 view threshold | 95,272.00 (-0.13%) | 95,843.10 (99.40%) | 📈 view plot 🚷 view threshold | 59,263.00 (-0.30%) | 59,713.73 (99.25%) | 📈 view plot 🚷 view threshold | 85,072.00 (-0.35%) | 85,819.25 (99.13%) | 📈 view plot 🚷 view threshold | 52.00 (+17.49%) | 59.83 (86.91%) | 📈 view plot 🚷 view threshold | 284.00 (+1.35%) | 290.19 (97.87%) |
| get_subscribe | 📈 view plot 🚷 view threshold | 7,859.00 (-1.89%) | 8,283.45 (94.88%) | 📈 view plot 🚷 view threshold | 2,758.00 (-2.40%) | 2,950.20 (93.49%) | 📈 view plot 🚷 view threshold | 3,834.00 (-2.86%) | 4,146.16 (92.47%) | 📈 view plot 🚷 view threshold | 14.00 (+18.36%) | 19.75 (70.89%) | 📈 view plot 🚷 view threshold | 113.00 (-1.24%) | 118.58 (95.29%) |
| serialize_authorize | 📈 view plot 🚷 view threshold | 12,177.00 (-0.78%) | 12,565.16 (96.91%) | 📈 view plot 🚷 view threshold | 5,240.00 (-1.36%) | 5,435.69 (96.40%) | 📈 view plot 🚷 view threshold | 7,277.00 (-1.72%) | 7,607.93 (95.65%) | 📈 view plot 🚷 view threshold | 14.00 (+47.98%) | 17.75 (78.86%) | 📈 view plot 🚷 view threshold | 138.00 (+0.18%) | 142.60 (96.78%) |
| serialize_deserialize_authorize | 📈 view plot 🚷 view threshold | 24,530.00 (-0.71%) | 25,206.11 (97.32%) | 📈 view plot 🚷 view threshold | 9,786.00 (-0.93%) | 10,037.08 (97.50%) | 📈 view plot 🚷 view threshold | 13,790.00 (-1.10%) | 14,201.45 (97.10%) | 📈 view plot 🚷 view threshold | 34.00 (-2.77%) | 45.09 (75.40%) | 📈 view plot 🚷 view threshold | 302.00 (-0.15%) | 314.08 (96.15%) |
| serialize_deserialize_handle_authorize | 📈 view plot 🚷 view threshold | 30,129.00 (-0.65%) | 30,752.59 (97.97%) | 📈 view plot 🚷 view threshold | 11,989.00 (-0.66%) | 12,207.80 (98.21%) | 📈 view plot 🚷 view threshold | 16,954.00 (-0.79%) | 17,312.37 (97.93%) | 📈 view plot 🚷 view threshold | 59.00 (+6.20%) | 67.25 (87.73%) | 📈 view plot 🚷 view threshold | 368.00 (-0.60%) | 380.60 (96.69%) |
| serialize_deserialize_handle_submit | 📈 view plot 🚷 view threshold | 126,456.00 (-0.01%) | 126,909.06 (99.64%) | 📈 view plot 🚷 view threshold | 73,117.00 (-0.21%) | 73,536.01 (99.43%) | 📈 view plot 🚷 view threshold | 104,761.00 (-0.27%) | 105,490.85 (99.31%) | 📈 view plot 🚷 view threshold | 111.00 (+4.90%) | 125.09 (88.74%) | 📈 view plot 🚷 view threshold | 604.00 (+1.17%) | 607.42 (99.44%) |
| serialize_deserialize_handle_subscribe | 📈 view plot 🚷 view threshold | 27,769.00 (-0.38%) | 28,528.08 (97.34%) | 📈 view plot 🚷 view threshold | 9,577.00 (-0.67%) | 9,764.59 (98.08%) | 📈 view plot 🚷 view threshold | 13,519.00 (-0.85%) | 13,833.98 (97.72%) | 📈 view plot 🚷 view threshold | 71.00 (+11.69%) | 76.03 (93.38%) | 📈 view plot 🚷 view threshold | 397.00 (-0.20%) | 414.04 (95.88%) |
| serialize_deserialize_submit | 📈 view plot 🚷 view threshold | 115,193.00 (-0.05%) | 115,780.64 (99.49%) | 📈 view plot 🚷 view threshold | 67,894.00 (-0.29%) | 68,416.76 (99.24%) | 📈 view plot 🚷 view threshold | 97,353.00 (-0.36%) | 98,270.89 (99.07%) | 📈 view plot 🚷 view threshold | 75.00 (+18.68%) | 81.63 (91.87%) | 📈 view plot 🚷 view threshold | 499.00 (+1.39%) | 500.93 (99.62%) |
| serialize_deserialize_subscribe | 📈 view plot 🚷 view threshold | 23,166.00 (-0.55%) | 23,918.48 (96.85%) | 📈 view plot 🚷 view threshold | 8,129.00 (-0.83%) | 8,320.56 (97.70%) | 📈 view plot 🚷 view threshold | 11,421.00 (-1.06%) | 11,741.37 (97.27%) | 📈 view plot 🚷 view threshold | 46.00 (+22.06%) | 47.29 (97.27%) | 📈 view plot 🚷 view threshold | 329.00 (-0.42%) | 345.31 (95.28%) |
| serialize_submit | 📈 view plot 🚷 view threshold | 99,733.00 (-0.09%) | 100,279.47 (99.46%) | 📈 view plot 🚷 view threshold | 61,325.00 (-0.26%) | 61,742.34 (99.32%) | 📈 view plot 🚷 view threshold | 87,928.00 (-0.33%) | 88,649.14 (99.19%) | 📈 view plot 🚷 view threshold | 58.00 (+24.19%) | 64.08 (90.51%) | 📈 view plot 🚷 view threshold | 329.00 (+1.24%) | 335.54 (98.05%) |
| serialize_subscribe | 📈 view plot 🚷 view threshold | 11,346.00 (-0.51%) | 11,674.43 (97.19%) | 📈 view plot 🚷 view threshold | 4,111.00 (-1.52%) | 4,291.97 (95.78%) | 📈 view plot 🚷 view threshold | 5,691.00 (-1.95%) | 5,998.92 (94.87%) | 📈 view plot 🚷 view threshold | 18.00 (+38.46%) | 21.64 (83.17%) | 📈 view plot 🚷 view threshold | 159.00 (+0.53%) | 163.77 (97.09%) |
|
| Branch | 2024-12-12-fix-template-provider-binary-permission |
| Testbed | sv2 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result nanoseconds (ns) (Result Δ%) | Upper Boundary nanoseconds (ns) (Limit %) |
|---|---|---|---|
| client_sv2_handle_message_common | 📈 view plot 🚷 view threshold | 44.22 (-0.70%) | 46.71 (94.67%) |
| client_sv2_handle_message_mining | 📈 view plot 🚷 view threshold | 74.74 (-2.07%) | 93.72 (79.75%) |
| client_sv2_mining_message_submit_standard | 📈 view plot 🚷 view threshold | 14.64 (-0.11%) | 14.76 (99.19%) |
| client_sv2_mining_message_submit_standard_serialize | 📈 view plot 🚷 view threshold | 259.74 (-2.55%) | 300.94 (86.31%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | 📈 view plot 🚷 view threshold | 607.14 (-1.58%) | 670.70 (90.52%) |
| client_sv2_open_channel | 📈 view plot 🚷 view threshold | 166.39 (+2.72%) | 187.97 (88.52%) |
| client_sv2_open_channel_serialize | 📈 view plot 🚷 view threshold | 296.06 (+4.06%) | 313.62 (94.40%) |
| client_sv2_open_channel_serialize_deserialize | 📈 view plot 🚷 view threshold | 375.57 (-2.13%) | 414.01 (90.72%) |
| client_sv2_setup_connection | 📈 view plot 🚷 view threshold | 157.54 (-1.80%) | 171.50 (91.86%) |
| client_sv2_setup_connection_serialize | 📈 view plot 🚷 view threshold | 466.02 (-0.90%) | 543.21 (85.79%) |
| client_sv2_setup_connection_serialize_deserialize | 📈 view plot 🚷 view threshold | 978.59 (-1.63%) | 1,077.29 (90.84%) |
|
| Branch | 2024-12-12-fix-template-provider-binary-permission |
| Testbed | sv2 |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Benchmark Result estimated cycles (Result Δ%) | Upper Boundary estimated cycles (Limit %) | Instructions | Benchmark Result instructions (Result Δ%) | Upper Boundary instructions (Limit %) | L1 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | L2 Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) | RAM Accesses | Benchmark Result accesses (Result Δ%) | Upper Boundary accesses (Limit %) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| client_sv2_handle_message_common | 📈 view plot 🚷 view threshold | 2,069.00 (-1.61%) | 2,216.85 (93.33%) | 📈 view plot 🚷 view threshold | 473.00 (-0.08%) | 488.79 (96.77%) | 📈 view plot 🚷 view threshold | 734.00 (-0.26%) | 757.50 (96.90%) | 📈 view plot 🚷 view threshold | 8.00 (+54.92%) | 13.19 (60.66%) | 📈 view plot 🚷 view threshold | 37.00 (-3.45%) | 41.40 (89.37%) |
| client_sv2_handle_message_mining | 📈 view plot 🚷 view threshold | 8,344.00 (+1.59%) | 8,355.55 (99.86%) | 📈 view plot 🚷 view threshold | 2,137.00 (-0.03%) | 2,143.08 (99.72%) | 📈 view plot 🚷 view threshold | 3,154.00 (-0.20%) | 3,171.20 (99.46%) | 📈 view plot 🚷 view threshold | 37.00 (+4.57%) | 41.35 (89.47%) | 📈 view plot 🚷 view threshold | 143.00 (+2.64%) | 143.26 (99.82%) |
| client_sv2_mining_message_submit_standard | 📈 view plot 🚷 view threshold | 6,319.00 (+0.42%) | 6,430.68 (98.26%) | 📈 view plot 🚷 view threshold | 1,750.00 (-0.05%) | 1,767.12 (99.03%) | 📈 view plot 🚷 view threshold | 2,549.00 (-0.15%) | 2,575.79 (98.96%) | 📈 view plot 🚷 view threshold | 19.00 (+11.71%) | 24.32 (78.13%) | 📈 view plot 🚷 view threshold | 105.00 (+0.56%) | 108.25 (97.00%) |
| client_sv2_mining_message_submit_standard_serialize | 📈 view plot 🚷 view threshold | 14,782.00 (+0.38%) | 14,945.11 (98.91%) | 📈 view plot 🚷 view threshold | 4,694.00 (-0.02%) | 4,711.12 (99.64%) | 📈 view plot 🚷 view threshold | 6,747.00 (-0.16%) | 6,784.52 (99.45%) | 📈 view plot 🚷 view threshold | 53.00 (+20.09%) | 57.38 (92.36%) | 📈 view plot 🚷 view threshold | 222.00 (+0.29%) | 226.92 (97.83%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | 📈 view plot 🚷 view threshold | 27,762.00 (+0.73%) | 27,932.59 (99.39%) | 📈 view plot 🚷 view threshold | 10,645.00 (+0.47%) | 10,674.46 (99.72%) | 📈 view plot 🚷 view threshold | 15,502.00 (+0.53%) | 15,552.43 (99.68%) | 📈 view plot 🚷 view threshold | 93.00 (+13.16%) | 94.12 (98.81%) | 📈 view plot 🚷 view threshold | 337.00 (+0.55%) | 341.97 (98.55%) |
| client_sv2_open_channel | 📈 view plot 🚷 view threshold | 4,447.00 (+1.36%) | 4,557.62 (97.57%) | 📈 view plot 🚷 view threshold | 1,461.00 (-0.03%) | 1,476.79 (98.93%) | 📈 view plot 🚷 view threshold | 2,157.00 (-0.16%) | 2,183.47 (98.79%) | 📈 view plot 🚷 view threshold | 10.00 (+19.18%) | 15.19 (65.83%) | 📈 view plot 🚷 view threshold | 64.00 (+2.52%) | 67.19 (95.25%) |
| client_sv2_open_channel_serialize | 📈 view plot 🚷 view threshold | 14,002.00 (-0.16%) | 14,253.28 (98.24%) | 📈 view plot 🚷 view threshold | 5,064.00 (-0.01%) | 5,079.79 (99.69%) | 📈 view plot 🚷 view threshold | 7,322.00 (-0.06%) | 7,350.80 (99.61%) | 📈 view plot 🚷 view threshold | 41.00 (+15.85%) | 45.00 (91.11%) | 📈 view plot 🚷 view threshold | 185.00 (-0.71%) | 192.98 (95.87%) |
| client_sv2_open_channel_serialize_deserialize | 📈 view plot 🚷 view threshold | 22,786.00 (+0.51%) | 22,949.87 (99.29%) | 📈 view plot 🚷 view threshold | 8,040.00 (+0.15%) | 8,062.23 (99.72%) | 📈 view plot 🚷 view threshold | 11,691.00 (+0.10%) | 11,728.98 (99.68%) | 📈 view plot 🚷 view threshold | 84.00 (+12.35%) | 86.16 (97.49%) | 📈 view plot 🚷 view threshold | 305.00 (+0.54%) | 310.48 (98.24%) |
| client_sv2_setup_connection | 📈 view plot 🚷 view threshold | 4,673.00 (-0.28%) | 4,782.85 (97.70%) | 📈 view plot 🚷 view threshold | 1,502.00 (-0.02%) | 1,517.79 (98.96%) | 📈 view plot 🚷 view threshold | 2,278.00 (-0.01%) | 2,299.31 (99.07%) | 📈 view plot 🚷 view threshold | 10.00 (+3.56%) | 16.49 (60.63%) | 📈 view plot 🚷 view threshold | 67.00 (-0.61%) | 70.02 (95.69%) |
| client_sv2_setup_connection_serialize | 📈 view plot 🚷 view threshold | 16,158.00 (+0.03%) | 16,357.18 (98.78%) | 📈 view plot 🚷 view threshold | 5,963.00 (-0.01%) | 5,978.79 (99.74%) | 📈 view plot 🚷 view threshold | 8,653.00 (-0.13%) | 8,690.81 (99.56%) | 📈 view plot 🚷 view threshold | 52.00 (+30.36%) | 53.43 (97.32%) | 📈 view plot 🚷 view threshold | 207.00 (-0.60%) | 213.53 (96.94%) |
| client_sv2_setup_connection_serialize_deserialize | 📈 view plot 🚷 view threshold | 35,680.00 (+0.32%) | 35,842.79 (99.55%) | 📈 view plot 🚷 view threshold | 14,888.00 (+0.19%) | 14,910.89 (99.85%) | 📈 view plot 🚷 view threshold | 21,870.00 (+0.17%) | 21,916.34 (99.79%) | 📈 view plot 🚷 view threshold | 109.00 (+18.03%) | 112.47 (96.91%) | 📈 view plot 🚷 view threshold | 379.00 (-0.07%) | 384.87 (98.48%) |
Shourya742
force-pushed
the
2024-12-12-fix-template-provider-binary-permission
branch
from
97140b4 to
971ee7b
Compare
|
@Shourya742 can you please edit the PR description with a detailed description of:
|
Done |
Shourya742
force-pushed
the
2024-12-12-fix-template-provider-binary-permission
branch
2 times, most recently
from
164be11 to
712e1dd
Compare
|
@Shourya742 it seems we changed the approach here? can we get a description of this new approach? (overall let's try to keep PRs descriptive, otherwise the review process becomes guesswork, which is not really ideal) |
|
tbh I'm not a fan of slapping therefore I feel it's important to have a detailed understanding on why we are doing this... and most importantly: why did we pivot away from the original approach? |
The reason why we pivoted from the original approach because it didn't worked. The issue was only happening when we were running the actions for the very first time when we open a PR (though via rerun it worked). I didn't get why using sudo is dirty??We use sudo because the tests access the /tmp directory, which encounters permission issues on GitHub's macOS runners. This ensures the necessary access to directories like /tmp during the workflow execution. This is the issue with macos runner, not our code. |
|
even tests that have nothing to do with the original problem (
slapping |
Right now, sudo is only applied to tests that somehow trigger roles/test, since the integration test is directly invoked when we run tests on roles. I’ve added sudo to the other Test actions just to keep the pattern consistent, but I can change that if it’s causing any confusion. |
|
the original solution description was:
our solution needs to encompass the scope of this problem without overescalating execution to we can achieve that with: - name: Roles Integration Tests (MAC)
if: matrix.os == 'macos-latest'
run: |
xattr -d com.apple.quarantine /tmp/.template-provider/bitcoin-sv2-tp-*/bin/bitcoin*
cargo test --manifest-path=roles/Cargo.toml --verbose --test '*' -- --nocapturethe the point here is that we are fixing the problem with a specific solution, instead of an umbrella with potential unintended consequences. |
This I did via code in my previous solution, but it didn't worked. |
it didn't work because the rust executable doesn't (and most importantly: shouldn't) have priviledged execution the suggestion above is to achieve this via |
Won't this fail, as the directory |
I see your point. Perhaps instead of calling - name: Roles Integration Tests (MAC)
if: matrix.os == 'macos-latest'
run: |
sudo mkdir -p /tmp/.template-provider
sudo chmod 777 /tmp/.template-provider
cargo test --manifest-path=roles/Cargo.toml --verbose --test '*' -- --nocapturethis approach tweaks permissions from a different perspective: we just create a then we shouldn't even need to tweak write permissions for the note: this is just a sketch, the commands could potentially look slightly different |
I observed another failure condition, apart from the one mentioned, which occurs during the prop test. Fortunately, I believe this is happening because of the latest macOS runner. I changed its version to 14, and now everything works. |
|
I think the current approach is much better. a few remarks:
|
Shourya742
force-pushed
the
2024-12-12-fix-template-provider-binary-permission
branch
from
c765ec6 to
6157916
Compare
Done and updated |
closes #1278
The error Permission denied (os error 13) indicates that your process does not have the necessary permissions to execute the bitcoind binary located at /tmp/.template-provider/bitcoin-sv2-tp-0.1.9/bin/bitcoind
Solution: changing macos runner version worked